Name the introducing dependency in PR/MR security comments

CHANGELOG.md

@@ -1,5 +1,18 @@
 # Changelog
 
+## 2.12.1
+
+### Added: the introducing dependency in pull request comments
+
+- Security and license alerts on a transitive dependency now name the direct
+  dependency that introduced it, under the manifest link. Up to five are listed.
+
+### Fixed: namespaces in introducing dependency names
+
+- Scoped npm packages and Maven artifacts named as the introducing dependency
+  keep their scope or group ID (`npm/@acme/widgets@1.2.0`, not
+  `npm/widgets@1.2.0`).
+
 ## 2.12.0
 
 ### Added

pyproject.toml

@@ -6,7 +6,7 @@ build-backend = "hatchling.build"
 
 [project]
 name = "socketsecurity"
-version = "2.12.0"
+version = "2.12.1"
 requires-python = ">= 3.11"
 license = {"file" = "LICENSE"}
 dependencies = [

socketsecurity/__init__.py

@@ -1,4 +1,4 @@
 __author__ = 'socket.dev'
-__version__ = '2.12.0'
+__version__ = '2.12.1'
 USER_AGENT_NAME = 'socket-python-cli'
 USER_AGENT = f'{USER_AGENT_NAME}/{__version__}'

socketsecurity/core/__init__.py

@@ -2681,7 +2681,12 @@ def get_source_data(package: Package, packages: dict) -> list:
                 top_package = packages.get(top_id)
                 if top_package:
                     manifests = ""
-                    top_purl = f"{top_package.type}/{top_package.name}@{top_package.version}"
+                    ancestor_name = (
+                        f"{top_package.namespace}/{top_package.name}"
+                        if top_package.namespace
+                        else top_package.name
+                    )
+                    top_purl = f"{top_package.type}/{ancestor_name}@{top_package.version}"
                     if hasattr(top_package, "manifestFiles") and top_package.manifestFiles:
                         for manifest_data in top_package.manifestFiles:
                             manifest_file = manifest_data.get("file")

socketsecurity/core/messages.py

@@ -844,6 +844,35 @@ def html_text(value) -> str:
         """
         return escape(Messages.inline_html_text(value))
 
+    # Enough to name the usual one or two ancestors without letting a package
+    # pulled in by dozens of direct dependencies swamp the alert row.
+    INTRODUCED_BY_LIMIT = 5
+
+    @staticmethod
+    def introduced_by_html(alert: Issue) -> str:
+        """Name the direct dependencies that pulled a transitive package in.
+
+        Returns an empty string for a direct dependency, or for a transitive one
+        whose ancestors are not in this scan, so the row only gains a line when
+        there is an ancestor to name.
+        """
+        ancestors = []
+        for entry in getattr(alert, "introduced_by", None) or []:
+            if not isinstance(entry, (list, tuple)) or not entry:
+                continue
+            source = entry[0]
+            if source and source != "direct" and source not in ancestors:
+                ancestors.append(source)
+        if not ancestors:
+            return ""
+
+        shown = ancestors[:Messages.INTRODUCED_BY_LIMIT]
+        names = ", ".join(f"<code>{Messages.html_text(name)}</code>" for name in shown)
+        hidden = len(ancestors) - len(shown)
+        if hidden:
+            names += f" and {hidden} more"
+        return f"<p><strong>Introduced by:</strong> {names}</p>"
+
     @staticmethod
     def html_attr(value) -> str:
         """Escape a value for an HTML attribute, quotes included.
@@ -1013,6 +1042,7 @@ def security_comment_template(diff: Diff, config=None) -> str:
       <p><strong>Note:</strong> {Messages.html_text(alert.description)}</p>
       {patched_version_html}
       <p><strong>Source:</strong> <a href="{Messages.html_attr(manifest_url)}">Manifest File</a></p>
+      {Messages.introduced_by_html(alert)}
       <p>ℹ️ Read more on:
       <a href="{Messages.html_attr(alert.purl)}">This package</a> |
       <a href="{Messages.html_attr(alert.url)}">This alert</a> |
@@ -1073,6 +1103,7 @@ def security_comment_template(diff: Diff, config=None) -> str:
             ) if show_ignore else ""
             comment += f"""      </ul>
       <p><strong>From:</strong> <a href="{Messages.html_attr(license_manifest_url)}">Manifest File</a></p>
+      {Messages.introduced_by_html(first_alert)}
       <p>ℹ️ Read more on: <a href="{Messages.html_attr(first_alert.purl)}">This package</a> | <a href="https://socket.dev/alerts/license">What is a license policy violation?</a></p>
       <blockquote>
         <p><em>Next steps:</em> Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at <strong>support@socket.dev</strong>.</p>

tests/unit/test_pr_comment_rendering.py

@@ -11,7 +11,8 @@
 
 import pytest
 
-from socketsecurity.core.classes import Comment, Diff, Issue
+from socketsecurity.core import Core
+from socketsecurity.core.classes import Comment, Diff, Issue, Package
 from socketsecurity.core.messages import Messages
 from socketsecurity.core.scm_comments import Comments
 
@@ -503,3 +504,129 @@ def test_plain_style_is_left_verbatim(self):
         manifest_str, _ = Messages.create_sources(alert, "plain")
 
         assert "<img src=x>/package.json" in manifest_str
+
+
+class TestIntroducedBy:
+    """The default comment names the direct dependency behind a transitive alert."""
+
+    def _body(self, alert: Issue) -> str:
+        return Messages.security_comment_template(_make_diff([alert]), _FakeConfig())
+
+    def test_transitive_alert_names_its_ancestor(self):
+        alert = _make_alert(
+            manifests="",
+            introduced_by=[("npm/@acme/widgets@1.2.0", "package.json")],
+        )
+
+        body = self._body(alert)
+
+        assert (
+            "<p><strong>Introduced by:</strong> <code>npm/@acme/widgets@1.2.0</code></p>"
+            in body
+        )
+        assert_html_block_intact(body)
+
+    def test_direct_dependency_gets_no_line(self):
+        alert = _make_alert(manifests="", introduced_by=[("direct", "package.json")])
+
+        body = self._body(alert)
+
+        assert "Introduced by" not in body
+        assert_html_block_intact(body)
+
+    def test_transitive_without_known_ancestors_gets_no_line(self):
+        body = self._body(_make_alert())
+
+        assert "Introduced by" not in body
+        assert_html_block_intact(body)
+
+    def test_repeated_ancestors_are_listed_once(self):
+        """One ancestor declared in two manifests yields two entries."""
+        alert = _make_alert(
+            manifests="",
+            introduced_by=[
+                ("npm/express@4.0.0", "a/package.json"),
+                ("npm/express@4.0.0", "b/package.json"),
+            ],
+        )
+
+        assert Messages.introduced_by_html(alert).count("npm/express@4.0.0") == 1
+
+    def test_long_ancestor_lists_are_capped(self):
+        limit = Messages.INTRODUCED_BY_LIMIT
+        alert = _make_alert(
+            manifests="",
+            introduced_by=[(f"npm/root-{i}@1.0.0", "package.json") for i in range(limit + 3)],
+        )
+
+        html = Messages.introduced_by_html(alert)
+
+        assert html.count("<code>") == limit
+        assert html.endswith(" and 3 more</p>")
+
+    def test_ancestor_name_is_escaped(self):
+        alert = _make_alert(
+            manifests="",
+            introduced_by=[("npm/<img src=x>@1.0.0", "package.json")],
+        )
+
+        body = self._body(alert)
+
+        assert "<img src=x>" not in body
+        assert "npm/&lt;img src=x&gt;@1.0.0" in body
+
+    def test_license_row_names_its_ancestor(self):
+        alert = _make_alert(
+            type="licenseSpdxDisj",
+            title="License Policy Violation",
+            manifests="",
+            introduced_by=[("pypi/requests@2.31.0", "requirements.txt")],
+        )
+
+        body = self._body(alert)
+
+        assert "has a License Policy Violation" in body
+        assert "<code>pypi/requests@2.31.0</code>" in body
+        assert_html_block_intact(body)
+
+
+def _package(package_id: str, **overrides):
+    fields = dict(
+        type="npm",
+        namespace=None,
+        name="widget",
+        version="1.0.0",
+        id=package_id,
+        score={},
+        alerts=[],
+        direct=False,
+        topLevelAncestors=[],
+        manifestFiles=[],
+        purl="",
+    )
+    fields.update(overrides)
+    return Package(**fields)
+
+
+def test_ancestor_labels_keep_their_namespace():
+    """Two scoped npm roots with the same name, and a Maven root, must render as
+    distinct, correctly named ancestors in the comment."""
+    packages = {
+        "one": _package("one", namespace="@one", manifestFiles=[{"file": "package.json"}]),
+        "two": _package("two", namespace="@two", manifestFiles=[{"file": "package.json"}]),
+        "mvn": _package(
+            "mvn", type="maven", namespace="com.acme", name="core",
+            manifestFiles=[{"file": "pom.xml"}],
+        ),
+        "plain": _package("plain", name="express", manifestFiles=[{"file": "package.json"}]),
+    }
+    child = _package("child", name="leaf", topLevelAncestors=["one", "two", "mvn", "plain"])
+
+    sources = [source for source, _ in Core.get_source_data(child, packages)]
+
+    assert sources == [
+        "npm/@one/widget@1.0.0",
+        "npm/@two/widget@1.0.0",
+        "maven/com.acme/core@1.0.0",
+        "npm/express@1.0.0",
+    ]