Skip to content

Commit 6f7abca

Browse files
committed
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.11.14.
1 parent bb77908 commit 6f7abca

7 files changed

Lines changed: 181 additions & 5 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,18 @@
11
# Changelog
22

3+
## 2.11.14
4+
5+
### Added: the introducing dependency in pull request comments
6+
7+
- Security and license alerts on a transitive dependency now name the direct
8+
dependency that introduced it, under the manifest link. Up to five are listed.
9+
10+
### Fixed: namespaces in introducing dependency names
11+
12+
- Scoped npm packages and Maven artifacts named as the introducing dependency
13+
keep their scope or group ID (`npm/@acme/widgets@1.2.0`, not
14+
`npm/widgets@1.2.0`).
15+
316
## 2.11.13
417

518
### Changed: `--repo` takes a repository slug

‎pyproject.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ build-backend = "hatchling.build"
66

77
[project]
88
name = "socketsecurity"
9-
version = "2.11.13"
9+
version = "2.11.14"
1010
requires-python = ">= 3.11"
1111
license = {"file" = "LICENSE"}
1212
dependencies = [

‎socketsecurity/__init__.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
__author__ = 'socket.dev'
2-
__version__ = '2.11.13'
2+
__version__ = '2.11.14'
33
USER_AGENT_NAME = 'socket-python-cli'
44
USER_AGENT = f'{USER_AGENT_NAME}/{__version__}'

‎socketsecurity/core/__init__.py‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2681,7 +2681,12 @@ def get_source_data(package: Package, packages: dict) -> list:
26812681
top_package = packages.get(top_id)
26822682
if top_package:
26832683
manifests = ""
2684-
top_purl = f"{top_package.type}/{top_package.name}@{top_package.version}"
2684+
ancestor_name = (
2685+
f"{top_package.namespace}/{top_package.name}"
2686+
if top_package.namespace
2687+
else top_package.name
2688+
)
2689+
top_purl = f"{top_package.type}/{ancestor_name}@{top_package.version}"
26852690
if hasattr(top_package, "manifestFiles") and top_package.manifestFiles:
26862691
for manifest_data in top_package.manifestFiles:
26872692
manifest_file = manifest_data.get("file")

‎socketsecurity/core/messages.py‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -844,6 +844,35 @@ def html_text(value) -> str:
844844
"""
845845
return escape(Messages.inline_html_text(value))
846846

847+
# Enough to name the usual one or two ancestors without letting a package
848+
# pulled in by dozens of direct dependencies swamp the alert row.
849+
INTRODUCED_BY_LIMIT = 5
850+
851+
@staticmethod
852+
def introduced_by_html(alert: Issue) -> str:
853+
"""Name the direct dependencies that pulled a transitive package in.
854+
855+
Returns an empty string for a direct dependency, or for a transitive one
856+
whose ancestors are not in this scan, so the row only gains a line when
857+
there is an ancestor to name.
858+
"""
859+
ancestors = []
860+
for entry in getattr(alert, "introduced_by", None) or []:
861+
if not isinstance(entry, (list, tuple)) or not entry:
862+
continue
863+
source = entry[0]
864+
if source and source != "direct" and source not in ancestors:
865+
ancestors.append(source)
866+
if not ancestors:
867+
return ""
868+
869+
shown = ancestors[:Messages.INTRODUCED_BY_LIMIT]
870+
names = ", ".join(f"<code>{Messages.html_text(name)}</code>" for name in shown)
871+
hidden = len(ancestors) - len(shown)
872+
if hidden:
873+
names += f" and {hidden} more"
874+
return f"<p><strong>Introduced by:</strong> {names}</p>"
875+
847876
@staticmethod
848877
def html_attr(value) -> str:
849878
"""Escape a value for an HTML attribute, quotes included.
@@ -1013,6 +1042,7 @@ def security_comment_template(diff: Diff, config=None) -> str:
10131042
<p><strong>Note:</strong> {Messages.html_text(alert.description)}</p>
10141043
{patched_version_html}
10151044
<p><strong>Source:</strong> <a href="{Messages.html_attr(manifest_url)}">Manifest File</a></p>
1045+
{Messages.introduced_by_html(alert)}
10161046
<p>ℹ️ Read more on:
10171047
<a href="{Messages.html_attr(alert.purl)}">This package</a> |
10181048
<a href="{Messages.html_attr(alert.url)}">This alert</a> |
@@ -1073,6 +1103,7 @@ def security_comment_template(diff: Diff, config=None) -> str:
10731103
) if show_ignore else ""
10741104
comment += f""" </ul>
10751105
<p><strong>From:</strong> <a href="{Messages.html_attr(license_manifest_url)}">Manifest File</a></p>
1106+
{Messages.introduced_by_html(first_alert)}
10761107
<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>
10771108
<blockquote>
10781109
<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‎

Lines changed: 128 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@
1111

1212
import pytest
1313

14-
from socketsecurity.core.classes import Comment, Diff, Issue
14+
from socketsecurity.core import Core
15+
from socketsecurity.core.classes import Comment, Diff, Issue, Package
1516
from socketsecurity.core.messages import Messages
1617
from socketsecurity.core.scm_comments import Comments
1718

@@ -503,3 +504,129 @@ def test_plain_style_is_left_verbatim(self):
503504
manifest_str, _ = Messages.create_sources(alert, "plain")
504505

505506
assert "<img src=x>/package.json" in manifest_str
507+
508+
509+
class TestIntroducedBy:
510+
"""The default comment names the direct dependency behind a transitive alert."""
511+
512+
def _body(self, alert: Issue) -> str:
513+
return Messages.security_comment_template(_make_diff([alert]), _FakeConfig())
514+
515+
def test_transitive_alert_names_its_ancestor(self):
516+
alert = _make_alert(
517+
manifests="",
518+
introduced_by=[("npm/@acme/widgets@1.2.0", "package.json")],
519+
)
520+
521+
body = self._body(alert)
522+
523+
assert (
524+
"<p><strong>Introduced by:</strong> <code>npm/@acme/widgets@1.2.0</code></p>"
525+
in body
526+
)
527+
assert_html_block_intact(body)
528+
529+
def test_direct_dependency_gets_no_line(self):
530+
alert = _make_alert(manifests="", introduced_by=[("direct", "package.json")])
531+
532+
body = self._body(alert)
533+
534+
assert "Introduced by" not in body
535+
assert_html_block_intact(body)
536+
537+
def test_transitive_without_known_ancestors_gets_no_line(self):
538+
body = self._body(_make_alert())
539+
540+
assert "Introduced by" not in body
541+
assert_html_block_intact(body)
542+
543+
def test_repeated_ancestors_are_listed_once(self):
544+
"""One ancestor declared in two manifests yields two entries."""
545+
alert = _make_alert(
546+
manifests="",
547+
introduced_by=[
548+
("npm/express@4.0.0", "a/package.json"),
549+
("npm/express@4.0.0", "b/package.json"),
550+
],
551+
)
552+
553+
assert Messages.introduced_by_html(alert).count("npm/express@4.0.0") == 1
554+
555+
def test_long_ancestor_lists_are_capped(self):
556+
limit = Messages.INTRODUCED_BY_LIMIT
557+
alert = _make_alert(
558+
manifests="",
559+
introduced_by=[(f"npm/root-{i}@1.0.0", "package.json") for i in range(limit + 3)],
560+
)
561+
562+
html = Messages.introduced_by_html(alert)
563+
564+
assert html.count("<code>") == limit
565+
assert html.endswith(" and 3 more</p>")
566+
567+
def test_ancestor_name_is_escaped(self):
568+
alert = _make_alert(
569+
manifests="",
570+
introduced_by=[("npm/<img src=x>@1.0.0", "package.json")],
571+
)
572+
573+
body = self._body(alert)
574+
575+
assert "<img src=x>" not in body
576+
assert "npm/&lt;img src=x&gt;@1.0.0" in body
577+
578+
def test_license_row_names_its_ancestor(self):
579+
alert = _make_alert(
580+
type="licenseSpdxDisj",
581+
title="License Policy Violation",
582+
manifests="",
583+
introduced_by=[("pypi/requests@2.31.0", "requirements.txt")],
584+
)
585+
586+
body = self._body(alert)
587+
588+
assert "has a License Policy Violation" in body
589+
assert "<code>pypi/requests@2.31.0</code>" in body
590+
assert_html_block_intact(body)
591+
592+
593+
def _package(package_id: str, **overrides):
594+
fields = dict(
595+
type="npm",
596+
namespace=None,
597+
name="widget",
598+
version="1.0.0",
599+
id=package_id,
600+
score={},
601+
alerts=[],
602+
direct=False,
603+
topLevelAncestors=[],
604+
manifestFiles=[],
605+
purl="",
606+
)
607+
fields.update(overrides)
608+
return Package(**fields)
609+
610+
611+
def test_ancestor_labels_keep_their_namespace():
612+
"""Two scoped npm roots with the same name, and a Maven root, must render as
613+
distinct, correctly named ancestors in the comment."""
614+
packages = {
615+
"one": _package("one", namespace="@one", manifestFiles=[{"file": "package.json"}]),
616+
"two": _package("two", namespace="@two", manifestFiles=[{"file": "package.json"}]),
617+
"mvn": _package(
618+
"mvn", type="maven", namespace="com.acme", name="core",
619+
manifestFiles=[{"file": "pom.xml"}],
620+
),
621+
"plain": _package("plain", name="express", manifestFiles=[{"file": "package.json"}]),
622+
}
623+
child = _package("child", name="leaf", topLevelAncestors=["one", "two", "mvn", "plain"])
624+
625+
sources = [source for source, _ in Core.get_source_data(child, packages)]
626+
627+
assert sources == [
628+
"npm/@one/widget@1.0.0",
629+
"npm/@two/widget@1.0.0",
630+
"maven/com.acme/core@1.0.0",
631+
"npm/express@1.0.0",
632+
]

‎uv.lock‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)