From b5e74b02be703a8a17ab3325dd6f7a1bb95a8eee Mon Sep 17 00:00:00 2001 From: Prachi Agrawal Date: Thu, 12 Nov 2026 21:46:15 +0530 Subject: [PATCH] Flag advisory commits not reachable from repository branches Signed-off-by: Prachi Agrawal --- docs/source/reference_improver_overview.rst | 28 +++ ..._packagecommitpatch_branch_reachability.py | 27 ++ vulnerabilities/models.py | 12 + .../v2_improvers/reference_collect_commits.py | 68 ++++- vulnerabilities/pipes/commit_reachability.py | 49 ++++ .../advisory_package_commit_details.html | 10 +- .../test_reference_collect_commits_v2.py | 236 +++++++++++++++++- ...est_vulnerablecode_importer_pipeline_v2.py | 6 +- vulnerabilities/views.py | 4 +- 9 files changed, 425 insertions(+), 15 deletions(-) create mode 100644 vulnerabilities/migrations/0145_packagecommitpatch_branch_reachability.py create mode 100644 vulnerabilities/pipes/commit_reachability.py diff --git a/docs/source/reference_improver_overview.rst b/docs/source/reference_improver_overview.rst index 417f5d8a2..5c9d1089d 100644 --- a/docs/source/reference_improver_overview.rst +++ b/docs/source/reference_improver_overview.rst @@ -32,3 +32,31 @@ like *Time Travel* and *finding fix commits* could be implemented as well. You can find more in-code documentation about improvers in :file:`vulnerabilites/improver.py` and the framework responsible for invoking these improvers in :file:`vulnerabilites/improve_runner.py`. + +Advisory commit branch reachability +----------------------------------- + +The ``collect_ref_fix_commits_v2`` pipeline collects commit references and then checks all +``PackageCommitPatch`` records, including commits collected directly by importers. Commits +and their advisory links are retained regardless of the check's result. + +The ``PackageCommitPatch.branch_reachability`` field records one of three outcomes: + +- ``reachable``: the commit is reachable from a branch in the referenced repository. +- ``unreachable``: a complete snapshot of all branch histories does not contain the commit. + A commit reachable only from a tag, a detached HEAD, or a pull request ref does not count + as belonging to a repository branch. +- ``unknown``: the commit has not been checked, the repository could not be cloned, or the + clone has incomplete history. Shallow and partial clones cannot establish unreachability. + +The check creates one temporary full bare clone per repository per run and removes it after +use. Clones include all branches and have a five-minute timeout; large repositories may +therefore remain unknown. A successful walk of all branch histories can establish +unreachability even when the referenced commit object is absent from the clone. + +The advisory's commit details page displays the result and highlights unreachable commits. +Pipeline logs include the repository URL and hash for these commits, for upstream review. +These results describe the branches observed during the last check and may change when +branches are added, deleted, or rewritten. Rerunning the pipeline refreshes existing records; +a failed check sets their status to unknown. The flag is local enrichment and is not part of +the source advisory's serialized patch data. diff --git a/vulnerabilities/migrations/0145_packagecommitpatch_branch_reachability.py b/vulnerabilities/migrations/0145_packagecommitpatch_branch_reachability.py new file mode 100644 index 000000000..9a0f157e3 --- /dev/null +++ b/vulnerabilities/migrations/0145_packagecommitpatch_branch_reachability.py @@ -0,0 +1,27 @@ +# Generated by Django 5.2.11 on 2026-11-12 16:04 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("vulnerabilities", "0144_remove_advisoryv2_advisory_latest_by_avid_idx_and_more"), + ] + + operations = [ + migrations.AddField( + model_name="packagecommitpatch", + name="branch_reachability", + field=models.CharField( + choices=[ + ("unknown", "Branch reachability unknown"), + ("reachable", "Reachable from a repository branch"), + ("unreachable", "Not reachable from any repository branch"), + ], + default="unknown", + help_text="Reachability from repository branches at the last check; unknown if incomplete.", + max_length=11, + ), + ), + ] diff --git a/vulnerabilities/models.py b/vulnerabilities/models.py index 521f0ca6a..b4219cb4f 100644 --- a/vulnerabilities/models.py +++ b/vulnerabilities/models.py @@ -2898,6 +2898,11 @@ class PackageCommitPatch(models.Model): A specific patch implementation for structured Package/VCS data """ + class BranchReachability(models.TextChoices): + UNKNOWN = "unknown", "Branch reachability unknown" + REACHABLE = "reachable", "Reachable from a repository branch" + UNREACHABLE = "unreachable", "Not reachable from any repository branch" + commit_hash = models.CharField( max_length=128, help_text="The commit hash of the patch in the VCS.", @@ -2908,6 +2913,13 @@ class PackageCommitPatch(models.Model): help_text="The Version Control System URL (e.g., git repo URL).", ) + branch_reachability = models.CharField( + max_length=11, + choices=BranchReachability.choices, + default=BranchReachability.UNKNOWN, + help_text="Reachability from repository branches at the last check; unknown if incomplete.", + ) + patch_text = models.TextField(blank=True, null=True) patch_checksum = models.CharField(max_length=128, blank=True, null=True) diff --git a/vulnerabilities/pipelines/v2_improvers/reference_collect_commits.py b/vulnerabilities/pipelines/v2_improvers/reference_collect_commits.py index c4fc908de..a06171483 100644 --- a/vulnerabilities/pipelines/v2_improvers/reference_collect_commits.py +++ b/vulnerabilities/pipelines/v2_improvers/reference_collect_commits.py @@ -6,11 +6,13 @@ # See https://github.com/aboutcode-org/vulnerablecode for support or download. # See https://aboutcode.org for more information about nexB OSS projects. # -from collections import defaultdict +from tempfile import TemporaryDirectory from aboutcode.federated import get_core_purl from aboutcode.pipeline import LoopProgress from django.db.models import Prefetch +from git import Repo +from git.exc import GitError from packageurl.contrib.purl2url import purl2url from packageurl.contrib.url2purl import url2purl @@ -20,6 +22,7 @@ from vulnerabilities.models import PackageCommitPatch from vulnerabilities.models import Patch from vulnerabilities.pipelines import VulnerableCodePipeline +from vulnerabilities.pipes.commit_reachability import get_branch_reachability from vulnerabilities.utils import is_commit @@ -32,14 +35,60 @@ class CollectReferencesFixCommitsPipeline(VulnerableCodePipeline): @classmethod def steps(cls): - return (cls.collect_and_store_fix_commits,) + return (cls.collect_and_store_fix_commits, cls.check_commit_branch_reachability) + + def check_commit_branch_reachability(self): + """Check all collected commits, cloning each repository once per run. + + Collection remains independent of repository availability. A fresh, full + bare clone includes all branch heads and their history, so even an object + absent from that clone can be reported as unreachable from its branches. + """ + vcs_urls = ( + PackageCommitPatch.objects.order_by("vcs_url") + .values_list("vcs_url", flat=True) + .distinct() + ) + for vcs_url in vcs_urls.iterator(): + commits = list( + PackageCommitPatch.objects.filter(vcs_url=vcs_url).only("id", "commit_hash") + ) + results = dict.fromkeys( + (commit.commit_hash for commit in commits), + PackageCommitPatch.BranchReachability.UNKNOWN, + ) + try: + with TemporaryDirectory(prefix="vcio-commit-branches-") as directory: + with Repo.clone_from( + vcs_url, + directory, + bare=True, + single_branch=False, + no_local=True, + no_tags=True, + env={"GIT_TERMINAL_PROMPT": "0"}, + kill_after_timeout=300, + ) as repo: + results = get_branch_reachability(repo, results, all_branches_fetched=True) + except (GitError, OSError, ValueError) as error: + self.log(f"Branch reachability unknown for {vcs_url}: {error}") + + for commit in commits: + commit.branch_reachability = results[commit.commit_hash] + if commit.branch_reachability == PackageCommitPatch.BranchReachability.UNREACHABLE: + self.log( + "Commit not reachable from repository branches: " + f"{vcs_url}@{commit.commit_hash}" + ) + + PackageCommitPatch.objects.bulk_update( + commits, fields=["branch_reachability"], batch_size=10000 + ) def get_vcs_data(self, url): - """Extracts a VCS URL and commit hash from URL. - >> get_vcs_commit('https://github.com/aboutcode-org/vulnerablecode/commit/98e516011d6e096e25247b82fc5f196bbeecff10') - ("pkg:github/aboutcode-org/vulnerablecode", 'https://github.com/aboutcode-org/vulnerablecode', '98e516011d6e096e25247b82fc5f196bbeecff10') - >> get_vcs_commit('https://github.com/aboutcode-org/vulnerablecode/pull/1974') - None + """Extract a core purl, VCS URL, and commit hash from a commit URL. + + Return None for non-commit URLs, including pull requests. """ try: purl = url2purl(url) @@ -54,7 +103,7 @@ def get_vcs_data(self, url): if base_purl and vcs_url and version: return base_purl, vcs_url, version except Exception as e: - self.log(f"Invalid URL: url:{url} error:{e}") + self.log(f"Invalid URL: url: {url} error: {e}") def collect_and_store_fix_commits(self): advisories = AdvisoryV2.objects.only("id").prefetch_related( @@ -85,7 +134,8 @@ def collect_and_store_fix_commits(self): updated_pkg_patch_commit_count += self.bulk_commit_batch_update(commit_batch) commit_batch.clear() - self.log(f"Successfully processed pkg patch commit {updated_pkg_patch_commit_count:,d}") + processed_count = format(updated_pkg_patch_commit_count, ",d") + self.log(f"Successfully processed pkg patch commit {processed_count}") def bulk_commit_batch_update(self, vcs_data_table): impact_data = {(row[0], row[3]) for row in vcs_data_table} # base_purl, adv_id diff --git a/vulnerabilities/pipes/commit_reachability.py b/vulnerabilities/pipes/commit_reachability.py new file mode 100644 index 000000000..c79f2bf42 --- /dev/null +++ b/vulnerabilities/pipes/commit_reachability.py @@ -0,0 +1,49 @@ +# +# Copyright (c) nexB Inc. and others. All rights reserved. +# VulnerableCode is a trademark of nexB Inc. +# SPDX-License-Identifier: Apache-2.0 +# See http://www.apache.org/licenses/LICENSE-2.0 for the license text. +# See https://github.com/aboutcode-org/vulnerablecode for support or download. +# See https://aboutcode.org for more information about nexB OSS projects. +# + +from vulnerabilities.models import PackageCommitPatch + + +def get_branch_reachability(repo, commit_hashes, *, all_branches_fetched=False): + """Return reachability for commits against a complete snapshot of branch heads. + + The caller must establish that all repository branches were fetched. Shallow + and partial clones cannot prove absence and yield unknown results. Tags, + detached HEADs, and pull request refs do not count as repository branches. + Git errors propagate so the caller can report unknown and retry later. + """ + status = PackageCommitPatch.BranchReachability + results = dict.fromkeys(commit_hashes, status.UNKNOWN) + if not all_branches_fetched or repo.git.rev_parse("--is-shallow-repository") == "true": + return results + + with repo.config_reader() as config: + if config.has_option("extensions", "partialclone"): + return results + for section in config.sections(): + if section.startswith('remote "') and config.get_value(section, "promisor", False): + return results + + # Traverse branch histories once, without fetching objects mentioned only in + # advisories. A missing object is unreachable only after this walk succeeds. + reachable = set(repo.git.rev_list("--branches", "--missing=error").splitlines()) + for commit_hash in results: + normalized = commit_hash.lower() + if len(normalized) in (40, 64): + results[commit_hash] = ( + status.REACHABLE if normalized in reachable else status.UNREACHABLE + ) + else: + matches = sum(commit.startswith(normalized) for commit in reachable) + if matches == 1: + results[commit_hash] = status.REACHABLE + elif matches == 0: + results[commit_hash] = status.UNREACHABLE + # An ambiguous abbreviated hash remains unknown. + return results diff --git a/vulnerabilities/templates/advisory_package_commit_details.html b/vulnerabilities/templates/advisory_package_commit_details.html index c1d0f360d..d6becd124 100644 --- a/vulnerabilities/templates/advisory_package_commit_details.html +++ b/vulnerabilities/templates/advisory_package_commit_details.html @@ -37,8 +37,11 @@ - {{ pkg_commit_patch.base_purl }}@{{ pkg_commit_patch.commit_hash }} + {{ impact.base_purl }}@{{ pkg_commit_patch.commit_hash }} + + {{ pkg_commit_patch.get_branch_reachability_display }} + @@ -51,6 +54,9 @@ {{ impact.base_purl }}@{{ pkg_commit_patch.commit_hash }} + + {{ pkg_commit_patch.get_branch_reachability_display }} + {% endfor %} @@ -72,4 +78,4 @@ -{% endblock %} \ No newline at end of file +{% endblock %} diff --git a/vulnerabilities/tests/pipelines/v2_improvers/test_reference_collect_commits_v2.py b/vulnerabilities/tests/pipelines/v2_improvers/test_reference_collect_commits_v2.py index 3ab1b4814..2158250bf 100644 --- a/vulnerabilities/tests/pipelines/v2_improvers/test_reference_collect_commits_v2.py +++ b/vulnerabilities/tests/pipelines/v2_improvers/test_reference_collect_commits_v2.py @@ -7,17 +7,250 @@ # See https://aboutcode.org for more information about nexB OSS projects. from datetime import datetime +from unittest.mock import patch import pytest +from django.template.loader import render_to_string +from django.test import RequestFactory +from git import Actor +from git import Repo +from git.exc import GitCommandError from vulnerabilities.models import AdvisoryReference from vulnerabilities.models import AdvisoryV2 from vulnerabilities.models import ImpactedPackage from vulnerabilities.models import PackageCommitPatch -from vulnerabilities.models import PackageV2 +from vulnerabilities.models import Patch from vulnerabilities.pipelines.v2_improvers.reference_collect_commits import ( CollectReferencesFixCommitsPipeline, ) +from vulnerabilities.pipes.commit_reachability import get_branch_reachability + + +@pytest.fixture +def local_git_repo(tmp_path): + """Create branch, tag-only, and dangling commits without any network access.""" + with Repo.init(tmp_path / "source", initial_branch="main") as repo: + actor = Actor("Reachability Test", "reachability@example.com") + + def commit(message): + return repo.index.commit(message, author=actor, committer=actor).hexsha + + root = commit("root") + main = commit("main") + repo.git.checkout("-b", "security", root) + security = commit("security branch only") + repo.git.checkout("--detach", root) + tagged = commit("tag only") + repo.create_tag("tagged-fix", tagged) + dangling = commit("dangling fix") + repo.git.checkout("main") + yield repo, { + "root": root, + "main": main, + "security": security, + "tagged": tagged, + "dangling": dangling, + } + + +@pytest.mark.parametrize( + "name,expected", + [ + ("root", "reachable"), + ("main", "reachable"), + ("security", "reachable"), + ("tagged", "unreachable"), + ("dangling", "unreachable"), + ], +) +def test_branch_reachability_uses_only_branches(local_git_repo, name, expected): + repo, commits = local_git_repo + commit_hash = commits[name] + assert get_branch_reachability(repo, [commit_hash], all_branches_fetched=True) == { + commit_hash: expected + } + + +def test_branch_reachability_requires_all_branches(local_git_repo, tmp_path): + source, commits = local_git_repo + with Repo.clone_from( + source.working_dir, tmp_path / "single", single_branch=True, branch="main" + ) as repo: + assert get_branch_reachability(repo, commits.values()) == dict.fromkeys( + commits.values(), "unknown" + ) + + +def test_branch_reachability_shallow_clone_is_unknown(local_git_repo, tmp_path): + source, commits = local_git_repo + with Repo.clone_from( + source.working_dir, tmp_path / "shallow", bare=True, depth=1, no_local=True + ) as repo: + assert repo.git.rev_parse("--is-shallow-repository") == "true" + assert get_branch_reachability( + repo, commits.values(), all_branches_fetched=True + ) == dict.fromkeys(commits.values(), "unknown") + + +def test_branch_reachability_partial_clone_is_unknown(local_git_repo, tmp_path): + source, commits = local_git_repo + with Repo.clone_from( + source.working_dir, tmp_path / "partial", bare=True, filter="blob:none" + ) as repo: + with repo.config_reader() as config: + assert config.get_value('remote "origin"', "promisor") is True + assert get_branch_reachability( + repo, commits.values(), all_branches_fetched=True + ) == dict.fromkeys(commits.values(), "unknown") + + +def test_branch_reachability_abbreviated_commit(local_git_repo): + repo, commits = local_git_repo + abbreviated = commits["security"][:12] + assert get_branch_reachability(repo, [abbreviated], all_branches_fetched=True) == { + abbreviated: "reachable" + } + + +@pytest.mark.django_db +def test_pipeline_preserves_commits_and_flags_reachability(local_git_repo): + repo, commits = local_git_repo + advisory = AdvisoryV2.objects.create( + advisory_id="CVE-2025-2367", + datasource_id="test-ds", + avid="test-ds/CVE-2025-2367", + url="https://example.com/advisory/CVE-2025-2367", + unique_content_id="reachability-test", + date_collected=datetime.now(), + pipeline_id="test-pipeline", + ) + vcs_url = repo.working_dir + references = [ + f"https://github.com/test/testpkg/commit/{commit_hash}" for commit_hash in commits.values() + ] + for url in references[:-1]: + advisory.references.add(AdvisoryReference.objects.create(url=url)) + # Patch references are collected as well as advisory references. + advisory.patches.add(Patch.objects.create(patch_url=references[-1])) + pipeline = CollectReferencesFixCommitsPipeline() + original_get_vcs_data = pipeline.get_vcs_data + + def local_vcs_data(url): + purl, _, commit_hash = original_get_vcs_data(url) + return purl, vcs_url, commit_hash + + with patch.object(pipeline, "get_vcs_data", side_effect=local_vcs_data): + pipeline.collect_and_store_fix_commits() + pipeline.collect_and_store_fix_commits() + + impact = advisory.impacted_packages.get() + assert impact.fixed_by_package_commit_patches.count() == len(commits) + assert set(PackageCommitPatch.objects.values_list("branch_reachability", flat=True)) == { + "unknown" + } + + # The repository is cloned once for all commits, and a rerun updates existing rows. + with patch( + "vulnerabilities.pipelines.v2_improvers.reference_collect_commits.Repo.clone_from", + wraps=Repo.clone_from, + ) as clone: + pipeline.check_commit_branch_reachability() + assert clone.call_count == 1 + + assert dict(PackageCommitPatch.objects.values_list("commit_hash", "branch_reachability")) == { + commit_hash: "reachable" if name in ("root", "main", "security") else "unreachable" + for name, commit_hash in commits.items() + } + assert impact.fixed_by_package_commit_patches.count() == len(commits) + assert any(commits["dangling"] in line for line in pipeline.execution_log) + + # Moving a branch to a previously dangling commit refreshes its flag. + repo.create_head("reviewed-fix", commits["dangling"]) + pipeline.check_commit_branch_reachability() + assert set(PackageCommitPatch.objects.values_list("branch_reachability", flat=True)) == { + "reachable" + } + + +@pytest.mark.django_db +def test_pipeline_marks_unavailable_repository_unknown(tmp_path, local_git_repo): + repo, commits = local_git_repo + available_commit = PackageCommitPatch.objects.create( + vcs_url=repo.working_dir, commit_hash=commits["main"] + ) + commit = PackageCommitPatch.objects.create( + vcs_url=str(tmp_path / "missing-repository"), + commit_hash="a" * 40, + branch_reachability="unreachable", + patch_text="preserve this patch", + ) + pipeline = CollectReferencesFixCommitsPipeline() + pipeline.check_commit_branch_reachability() + commit.refresh_from_db() + assert commit.branch_reachability == "unknown" + assert commit.patch_text == "preserve this patch" + available_commit.refresh_from_db() + assert available_commit.branch_reachability == "reachable" + assert any("Branch reachability unknown" in line for line in pipeline.execution_log) + + +@pytest.mark.django_db +def test_pipeline_marks_shallow_repository_unknown(local_git_repo, tmp_path): + source, commits = local_git_repo + with Repo.clone_from( + source.working_dir, tmp_path / "shallow-source", bare=True, depth=1, no_local=True + ) as shallow: + commit = PackageCommitPatch.objects.create( + vcs_url=shallow.git_dir, commit_hash=commits["root"] + ) + CollectReferencesFixCommitsPipeline().check_commit_branch_reachability() + commit.refresh_from_db() + assert commit.branch_reachability == "unknown" + + +@pytest.mark.django_db +def test_pipeline_git_walk_failure_is_unknown(local_git_repo): + repo, commits = local_git_repo + commit = PackageCommitPatch.objects.create( + vcs_url=repo.working_dir, commit_hash=commits["main"] + ) + with patch( + "vulnerabilities.pipelines.v2_improvers.reference_collect_commits.get_branch_reachability", + side_effect=GitCommandError("git rev-list --branches", 128), + ): + CollectReferencesFixCommitsPipeline().check_commit_branch_reachability() + commit.refresh_from_db() + assert commit.branch_reachability == "unknown" + + +@pytest.mark.django_db +@pytest.mark.parametrize("status", ["reachable", "unreachable", "unknown"]) +def test_advisory_commit_details_display_reachability(status): + advisory = AdvisoryV2.objects.create( + advisory_id="CVE-2025-2367", + datasource_id="test-ds", + avid="test-ds/CVE-2025-2367", + url="https://example.com/advisory/CVE-2025-2367", + unique_content_id="reachability-display", + date_collected=datetime.now(), + pipeline_id="test-pipeline", + ) + impact = ImpactedPackage.objects.create(advisory=advisory, base_purl="pkg:github/test/testpkg") + commit = PackageCommitPatch.objects.create( + vcs_url="https://github.com/test/testpkg", + commit_hash="a" * 40, + branch_reachability=status, + ) + impact.fixed_by_package_commit_patches.add(commit) + impact.introduced_by_package_commit_patches.add(commit) + html = render_to_string( + "advisory_package_commit_details.html", + {"advisoryv2": advisory}, + request=RequestFactory().get("/"), + ) + assert html.count(commit.get_branch_reachability_display()) == 2 + assert ("is-warning" in html) == (status == "unreachable") @pytest.mark.django_db @@ -49,6 +282,7 @@ def test_collect_fix_commits_pipeline_creates_entry(): fix = package_commit_patch.first() assert fix.commit_hash == "6bd301819f8f69331a55ae2336c8b111fc933f3d" assert fix.vcs_url == "https://github.com/test/testpkg" + assert fix.branch_reachability == "unknown" assert impacted_packages.first().fixed_by_package_commit_patches.count() == 1 diff --git a/vulnerabilities/tests/pipes/test_vulnerablecode_importer_pipeline_v2.py b/vulnerabilities/tests/pipes/test_vulnerablecode_importer_pipeline_v2.py index add78b06c..7c500672d 100644 --- a/vulnerabilities/tests/pipes/test_vulnerablecode_importer_pipeline_v2.py +++ b/vulnerabilities/tests/pipes/test_vulnerablecode_importer_pipeline_v2.py @@ -278,7 +278,7 @@ def test_patch_advisory(dumpy_patch_advisory): package_commit_patch.patch_text, package_commit_patch.patch_checksum, ) - for package_commit_patch in PackageCommitPatch.objects.all() + for package_commit_patch in PackageCommitPatch.objects.order_by("-commit_hash") ] == [ ( "be891173be2fbdc897116bf5aa4fc9fdc8dc4f3d", @@ -294,6 +294,10 @@ def test_patch_advisory(dumpy_patch_advisory): ), ] + assert set(PackageCommitPatch.objects.values_list("branch_reachability", flat=True)) == { + "unknown" + } + pkg_patches_that_fix = ( PackageCommitPatch.objects.filter(fixed_in_impacts__isnull=False).distinct().count() ) diff --git a/vulnerabilities/views.py b/vulnerabilities/views.py index b5ba74214..2619a77e0 100644 --- a/vulnerabilities/views.py +++ b/vulnerabilities/views.py @@ -980,13 +980,13 @@ def get_queryset(self): Prefetch( "introduced_by_package_commit_patches", queryset=models.PackageCommitPatch.objects.only( - "id", "commit_hash", "vcs_url" + "id", "commit_hash", "vcs_url", "branch_reachability" ), ), Prefetch( "fixed_by_package_commit_patches", queryset=models.PackageCommitPatch.objects.only( - "id", "commit_hash", "vcs_url" + "id", "commit_hash", "vcs_url", "branch_reachability" ), ), ),