From 9b1a282c9fa134f6ebcc8231f3d51efa16ec72f2 Mon Sep 17 00:00:00 2001 From: jdalton Date: Tue, 13 Oct 2026 14:04:26 -0400 Subject: [PATCH] Correct package and alert information links Use package destinations and alert-specific documentation in JSON and security comments. Preserve existing alert fields and omit links when their source value is missing. Assisted-by: Codex:GPT-6 --- docs/cli-reference.md | 8 ++ socketsecurity/core/messages.py | 35 ++++- socketsecurity/core/report_output.py | 10 ++ socketsecurity/core/report_urls.py | 22 ++- .../test_json_package_changes_transport.py | 16 +++ tests/unit/test_report_output.py | 82 +++++++++++ tests/unit/test_report_urls.py | 131 +++++++++++++++++- 7 files changed, 296 insertions(+), 8 deletions(-) diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 4b2d91b..aa75ff6 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -341,6 +341,14 @@ from `report_url`, which is the display link and may include output options such as license-detail settings. These additions apply to standard Socket JSON, not FOSSA compatibility output. +Each selected alert in standard Socket JSON also includes `package_url` and +`info_url`. `package_url` preserves the existing alert `url` value, which is a +package overview link; `info_url` links to documentation for the alert's type. +Either field is JSON null when its source value is unavailable. The existing +`url` and `purl` fields retain their meanings and values. These per-alert links +are distinct from the scan-reference URLs above and do not identify an +individual alert or triage page. + Standard Socket JSON also includes four package-change arrays: `new_packages`, `updated_packages`, `removed_packages`, and `replaced_packages`. Rows contain `id`, `name`, `version`, `ecosystem`, `direct`, `author`, `size`, diff --git a/socketsecurity/core/messages.py b/socketsecurity/core/messages.py index 26c2460..91e6177 100644 --- a/socketsecurity/core/messages.py +++ b/socketsecurity/core/messages.py @@ -6,12 +6,13 @@ from datetime import datetime, timezone from html import escape from pathlib import Path +from typing import Optional from mdutils import MdUtils from prettytable import PrettyTable from socketsecurity.core.classes import Diff, Issue, Purl -from socketsecurity.core.report_urls import add_query_params +from socketsecurity.core.report_urls import add_query_params, build_alert_links log = logging.getLogger("socketcli") @@ -945,6 +946,25 @@ def get_view_report_url(diff: Diff) -> str: return diff.report_url return "" + @staticmethod + def _format_alert_links( + package_url: Optional[str], alert_type: Optional[str] + ) -> str: + links = build_alert_links(package_url, alert_type) + anchors: list[str] = [] + if links.package_url: + anchors.append( + f'This package' + ) + if links.info_url: + anchors.append( + f'' + "About this alert type" + ) + if not anchors: + return "" + return f"

ℹ️ Read more on: {' | '.join(anchors)}

" + @staticmethod def security_comment_template(diff: Diff, config=None) -> str: """ @@ -1011,6 +1031,9 @@ def security_comment_template(diff: Diff, config=None) -> str: ) # Generate proper manifest URL manifest_url = Messages.get_manifest_file_url(diff, alert.manifests, config) + alert_links = Messages._format_alert_links( + getattr(alert, "url", None), alert.type + ) pkg_label = Messages.html_text(f"{alert.pkg_name}@{alert.pkg_version}") pkg_marker = Messages.comment_marker_text(f"{alert.pkg_name}@{alert.pkg_version}") # Generate a table row for each alert @@ -1033,10 +1056,7 @@ def security_comment_template(diff: Diff, config=None) -> str:

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

{patched_version_html}

Source: Manifest File

-

ℹ️ Read more on: - This package | - This alert | - What is known malware?

+ {alert_links}

Suggestion: {Messages.html_text(alert.suggestion)}

{ignore_html} @@ -1084,6 +1104,9 @@ def security_comment_template(diff: Diff, config=None) -> str: # Generate proper manifest URL for license violations license_manifest_url = Messages.get_manifest_file_url(diff, first_alert.manifests, config) + alert_links = Messages._format_alert_links( + getattr(first_alert, "url", None), first_alert.type + ) license_ignore_html = ( f"

Mark the package as acceptable risk: To ignore this alert only in this pull request, reply with the comment " @@ -1093,7 +1116,7 @@ def security_comment_template(diff: Diff, config=None) -> str: ) if show_ignore else "" comment += f"""

From: Manifest File

-

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

+ {alert_links}

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.

Suggestion: Find a package that does not violate your license policy or adjust your policy to allow this package's license.

diff --git a/socketsecurity/core/report_output.py b/socketsecurity/core/report_output.py index e749b2d..b5b9b45 100644 --- a/socketsecurity/core/report_output.py +++ b/socketsecurity/core/report_output.py @@ -9,6 +9,7 @@ ) from socketsecurity.core.classes import Diff, Purl from socketsecurity.core.messages import Messages +from socketsecurity.core.report_urls import build_alert_links @dataclass(frozen=True) @@ -141,6 +142,15 @@ def build_socket_json_report( ) selected_diff = clone_diff_with_selected_alerts(diff_report, selected_alerts) report = Messages.create_security_comment_json(selected_diff) + for alert_row in report["new_alerts"]: + package_url = alert_row.get("url") + alert_type = alert_row.get("type") + links = build_alert_links( + package_url if isinstance(package_url, str) else None, + alert_type if isinstance(alert_type, str) else None, + ) + alert_row["package_url"] = links.package_url + alert_row["info_url"] = links.info_url legal_flag = getattr(config, "legal", False) repo = getattr(config, "repo", None) branch = getattr(config, "branch", None) diff --git a/socketsecurity/core/report_urls.py b/socketsecurity/core/report_urls.py index f7880e7..95e606c 100644 --- a/socketsecurity/core/report_urls.py +++ b/socketsecurity/core/report_urls.py @@ -1,5 +1,25 @@ +from dataclasses import dataclass from typing import Mapping, Optional -from urllib.parse import parse_qsl, urlencode, urlsplit, urlunsplit +from urllib.parse import parse_qsl, quote, urlencode, urlsplit, urlunsplit + + +@dataclass(frozen=True) +class AlertLinks: + package_url: Optional[str] + info_url: Optional[str] + + +def build_alert_links( + package_url: Optional[str], alert_type: Optional[str] +) -> AlertLinks: + return AlertLinks( + package_url=package_url or None, + info_url=( + f"https://socket.dev/alerts/{quote(alert_type, safe='')}" + if alert_type + else None + ), + ) def add_query_params( diff --git a/tests/core/test_json_package_changes_transport.py b/tests/core/test_json_package_changes_transport.py index b827483..2171fba 100644 --- a/tests/core/test_json_package_changes_transport.py +++ b/tests/core/test_json_package_changes_transport.py @@ -487,12 +487,28 @@ def test_transitive_alert_and_package_json_keep_ancestor_namespaces( assert alert_diff.new_alerts[0].introduced_by == [ tuple(row) for row in expected_sources ] + issue = alert_diff.new_alerts[0] + expected_package_url = "https://socket.dev/pypi/package/leaf/overview/2.0.0" + assert issue.url == expected_package_url _, source_text = Messages.create_sources(alert_diff.new_alerts[0], "raw") assert source_text == "npm/@one/widget@1.0.0;npm/@two/widget@1.0.0" + comment = Messages.security_comment_template(alert_diff) + assert f'This package' in comment + assert ( + 'About this alert type' + in comment + ) + assert f'href="{issue.purl}"' not in comment alert_report = build_socket_json_report( alert_diff, CliConfig(api_token="fictional-local-only", repo="fixture-repo") ) assert alert_report["new_alerts"][0]["introduced_by"] == expected_sources + assert alert_report["new_alerts"][0]["url"] == issue.url + assert alert_report["new_alerts"][0]["purl"] == issue.purl + assert alert_report["new_alerts"][0]["package_url"] == expected_package_url + assert alert_report["new_alerts"][0]["info_url"] == ( + "https://socket.dev/alerts/envVars" + ) assert alert_report["scan_failed"] is False full_diff = core.create_diff_report( diff --git a/tests/unit/test_report_output.py b/tests/unit/test_report_output.py index d07b349..ff2d4a9 100644 --- a/tests/unit/test_report_output.py +++ b/tests/unit/test_report_output.py @@ -671,3 +671,85 @@ def test_full_scan_package_inventory_does_not_become_change_lists(): assert report["updated_packages"] == [] assert report["removed_packages"] == [] assert report["replaced_packages"] == [] + + +def test_alert_json_links_are_additive_and_do_not_mutate_real_models(): + issue = _issue("envVars", warn=True) + issue.url = "https://packages.example.test/@scope/pkg?view=a&mode=b#source" + issue.purl = "pkg:npm/@scope/pkg@1.2.3" + issue.props = {"nested": {"value": ["kept"]}} + diff = _diff(new_alerts=[issue]) + diff_before = copy.deepcopy(diff.__dict__) + issue_before = copy.deepcopy(issue.__dict__) + original_alert = json.loads(str(issue)) + + report = build_socket_json_report(diff, _config()) + serialized = json.loads(json.dumps(report)) + alert_row = serialized["new_alerts"][0] + + assert alert_row == { + **original_alert, + "package_url": "https://packages.example.test/@scope/pkg?view=a&mode=b#source", + "info_url": "https://socket.dev/alerts/envVars", + } + assert alert_row["url"] == original_alert["url"] + assert alert_row["purl"] == "pkg:npm/@scope/pkg@1.2.3" + assert set(report) == { + "scan_failed", + "new_alerts", + "full_scan_id", + "diff_url", + "report_url", + "diff_scan_id", + "old_scan_id", + "new_scan_id", + "old_scan_url", + "new_scan_url", + "repo", + "branch", + "commit_sha", + "legal_mode", + "new_packages", + "updated_packages", + "removed_packages", + "replaced_packages", + } + assert len(report) == 18 + assert diff.__dict__ == diff_before + assert issue.__dict__ == issue_before + assert "package_url" not in issue.__dict__ + assert "info_url" not in issue.__dict__ + + +def test_alert_json_links_follow_existing_selection_and_emit_nulls(): + new_alerts = [_issue("envVars", warn=True), _issue("highCVE", warn=True)] + unchanged = _issue("unchangedType", warn=True) + del new_alerts[1].url + unchanged.type = "" + unchanged.url = "" + diff = _diff(new_alerts=new_alerts, unchanged_alerts=[unchanged]) + + ordinary = json.loads(json.dumps(build_socket_json_report(diff, _config()))) + strict = json.loads( + json.dumps(build_socket_json_report(diff, _config(strict_blocking=True))) + ) + + assert [row["type"] for row in ordinary["new_alerts"]] == ["envVars", "highCVE"] + assert [row["type"] for row in strict["new_alerts"]] == [ + "envVars", + "highCVE", + "", + ] + assert ordinary["new_alerts"][0]["info_url"] == ( + "https://socket.dev/alerts/envVars" + ) + assert "url" not in ordinary["new_alerts"][1] + assert ordinary["new_alerts"][1]["package_url"] is None + assert ordinary["new_alerts"][1]["info_url"] == ( + "https://socket.dev/alerts/highCVE" + ) + assert strict["new_alerts"][2]["package_url"] is None + assert strict["new_alerts"][2]["info_url"] is None + assert ordinary["scan_failed"] is False + assert strict["scan_failed"] is False + assert build_socket_json_report(_diff(), _config())["new_alerts"] == [] diff --git a/tests/unit/test_report_urls.py b/tests/unit/test_report_urls.py index 72d7d17..9df458d 100644 --- a/tests/unit/test_report_urls.py +++ b/tests/unit/test_report_urls.py @@ -2,11 +2,140 @@ from socketsecurity.core.classes import Diff, Issue from socketsecurity.core.diff_result import matching_full_scan_url, normalize_report_url from socketsecurity.core.messages import Messages -from socketsecurity.core.report_urls import add_query_params +from socketsecurity.core.report_urls import add_query_params, build_alert_links from socketsecurity.plugins.jira import JiraPlugin from socketsecurity.plugins.slack import SlackPlugin +def test_alert_link_projection_preserves_package_url_and_encodes_type_segment(): + package_url = "https://packages.example.test/@scope/pkg?view=a&mode=b#source" + assert build_alert_links(package_url, "envVars").package_url == package_url + assert build_alert_links(package_url, "envVars").info_url == ( + "https://socket.dev/alerts/envVars" + ) + assert build_alert_links(package_url, "highCVE").info_url == ( + "https://socket.dev/alerts/highCVE" + ) + assert build_alert_links(None, None).package_url is None + assert build_alert_links(None, None).info_url is None + assert build_alert_links("", "").package_url is None + assert build_alert_links("", "").info_url is None + assert build_alert_links("https://pkg.example/a", "odd/type?#").info_url == ( + "https://socket.dev/alerts/odd%2Ftype%3F%23" + ) + + +def _security_issue( + alert_type: str | None, + package_url: str | None, + *, + package_name: str = "sample", +) -> Issue: + return Issue( + pkg_name=package_name, + pkg_version="1.0.0", + pkg_type="npm", + type=alert_type, + severity="high", + title="Fixture finding", + description="Fixture description", + suggestion="Upgrade", + purl="pkg:npm/identifier-only@1.0.0", + url=package_url, + manifests="package.json", + introduced_by=[], + error=True, + props={}, + ) + + +def test_security_comment_uses_package_and_type_documentation_links(): + issue = _security_issue( + "envVars", + 'https://packages.example.test/@scope/pkg?view="a"&mode=b', + ) + comment = Messages.security_comment_template( + Diff(new_alerts=[issue], report_url="https://reports.example.test/scan") + ) + + assert ( + 'This package' + in comment + ) + assert ( + 'About this alert type' + in comment + ) + assert "pkg:npm/identifier-only@1.0.0" not in comment + assert "alerts/malware" not in comment + assert "What is known malware?" not in comment + assert "This alert" not in comment + assert "start-socket-alert-sample" in comment + assert "package.json" in comment + assert "View full report" in comment + + malware_comment = Messages.security_comment_template( + Diff(new_alerts=[_security_issue("malware", "https://pkg.example.test/a")]) + ) + assert ( + 'About this alert type' + in malware_comment + ) + assert "What is known malware?" not in malware_comment + + +def test_security_comment_license_and_missing_alert_links(): + license_issue = _security_issue( + "licenseSpdxDisj", + "https://packages.example.test/license", + package_name="licensed-package", + ) + license_comment = Messages.security_comment_template( + Diff(new_alerts=[license_issue]) + ) + assert ( + 'This package' + in license_comment + ) + assert ( + 'About this alert type' + in license_comment + ) + + comment = Messages.security_comment_template( + Diff( + new_alerts=[ + _security_issue(None, "https://pkg.example.test/no-type"), + _security_issue("envVars", None, package_name="no-package-url"), + _security_issue(None, None, package_name="no-links"), + ] + ) + ) + assert 'This package' in comment + assert ( + 'About this alert type' + in comment + ) + assert " |

" not in comment + assert comment.count("ℹ️ Read more on:") == 2 + assert all( + 'href=""' not in line + for line in comment.splitlines() + if "Read more on:" in line + ) + + missing_package_url = _security_issue("envVars", None, package_name="missing-url") + del missing_package_url.url + missing_url_comment = Messages.security_comment_template( + Diff(new_alerts=[missing_package_url]) + ) + assert ( + 'About this alert type' + in missing_url_comment + ) + assert "This package" not in missing_url_comment + + def test_url_normalization_and_nested_scan_identity(): assert normalize_report_url(" https://report.invalid/a?q=1#part ") == ( " https://report.invalid/a?q=1#part "