From 84f06bc1bd98b4cd8912f96167561d0dec027fe7 Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Fri, 13 Nov 2026 20:55:59 -0500 Subject: [PATCH] Name the introducing dependency in PR/MR security comments Security and license alerts on a transitive dependency now show the direct dependency that introduced it, under the manifest link. The default comment previously showed only the manifest file; introduced_by was rendered only in the legacy table format. Ancestors are deduplicated and capped at five. Keep the namespace in introducing package labels, so scoped npm packages and Maven artifacts render as npm/@scope/name@version rather than losing their scope or group ID. Bump to 2.12.1. --- CHANGELOG.md | 13 +++ pyproject.toml | 2 +- socketsecurity/__init__.py | 2 +- socketsecurity/core/__init__.py | 7 +- socketsecurity/core/messages.py | 31 ++++++ tests/unit/test_pr_comment_rendering.py | 129 +++++++++++++++++++++++- uv.lock | 2 +- 7 files changed, 181 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 810aabd4..d81238f0 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/pyproject.toml b/pyproject.toml index b62bd60f..3ee6508e 100644 --- a/pyproject.toml +++ b/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 = [ diff --git a/socketsecurity/__init__.py b/socketsecurity/__init__.py index 203f24cf..01e27579 100644 --- a/socketsecurity/__init__.py +++ b/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__}' diff --git a/socketsecurity/core/__init__.py b/socketsecurity/core/__init__.py index f78f6c19..8cfd1e9c 100644 --- a/socketsecurity/core/__init__.py +++ b/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") diff --git a/socketsecurity/core/messages.py b/socketsecurity/core/messages.py index b2229f65..3607c9f9 100644 --- a/socketsecurity/core/messages.py +++ b/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"{Messages.html_text(name)}" for name in shown) + hidden = len(ancestors) - len(shown) + if hidden: + names += f" and {hidden} more" + return f"

Introduced by: {names}

" + @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:

Note: {Messages.html_text(alert.description)}

{patched_version_html}

Source: Manifest File

+ {Messages.introduced_by_html(alert)}

ℹ️ Read more on: This package | This alert | @@ -1073,6 +1103,7 @@ def security_comment_template(diff: Diff, config=None) -> str: ) if show_ignore else "" comment += f"""

From: Manifest File

+ {Messages.introduced_by_html(first_alert)}

ℹ️ Read more on: This package | What is a license policy violation?

Next steps: 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 support@socket.dev.

diff --git a/tests/unit/test_pr_comment_rendering.py b/tests/unit/test_pr_comment_rendering.py index 78c2a567..b28dd8e3 100644 --- a/tests/unit/test_pr_comment_rendering.py +++ b/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 "/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 ( + "

Introduced by: npm/@acme/widgets@1.2.0

" + 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("") == limit + assert html.endswith(" and 3 more

") + + def test_ancestor_name_is_escaped(self): + alert = _make_alert( + manifests="", + introduced_by=[("npm/@1.0.0", "package.json")], + ) + + body = self._body(alert) + + assert "" not in body + assert "npm/<img src=x>@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 "pypi/requests@2.31.0" 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", + ] diff --git a/uv.lock b/uv.lock index dc276bb5..6077c132 100644 --- a/uv.lock +++ b/uv.lock @@ -1334,7 +1334,7 @@ wheels = [ [[package]] name = "socketsecurity" -version = "2.12.0" +version = "2.12.1" source = { editable = "." } dependencies = [ { name = "beautifulsoup4" },