From 6b113c03db57820a146fedb7fe02a6d9257797c7 Mon Sep 17 00:00:00 2001 From: jdalton Date: Mon, 12 Oct 2026 18:32:21 -0400 Subject: [PATCH] Fix report links in Console and JSON Assisted-by: Codex:gpt-6-luna --- pyproject.toml | 2 +- socketsecurity/__init__.py | 2 +- socketsecurity/core/report_output.py | 140 ++++++++++++++ socketsecurity/output.py | 80 +------- tests/unit/test_report_output.py | 271 +++++++++++++++++++++++++++ uv.lock | 2 +- 6 files changed, 417 insertions(+), 80 deletions(-) create mode 100644 socketsecurity/core/report_output.py create mode 100644 tests/unit/test_report_output.py diff --git a/pyproject.toml b/pyproject.toml index 36d7e47..0b5c1ba 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -6,7 +6,7 @@ build-backend = "hatchling.build" [project] name = "socketsecurity" -version = "2.10.11" +version = "2.10.12" requires-python = ">= 3.11" license = {"file" = "LICENSE"} dependencies = [ diff --git a/socketsecurity/__init__.py b/socketsecurity/__init__.py index a057239..025b61a 100644 --- a/socketsecurity/__init__.py +++ b/socketsecurity/__init__.py @@ -1,3 +1,3 @@ __author__ = 'socket.dev' -__version__ = '2.10.11' +__version__ = '2.10.12' USER_AGENT = f'SocketPythonCLI/{__version__}' diff --git a/socketsecurity/core/report_output.py b/socketsecurity/core/report_output.py new file mode 100644 index 0000000..5095865 --- /dev/null +++ b/socketsecurity/core/report_output.py @@ -0,0 +1,140 @@ +from dataclasses import dataclass +from typing import Optional + +from socketsecurity.config import CliConfig +from socketsecurity.core.alert_selection import ( + clone_diff_with_selected_alerts, + select_diff_alerts, +) +from socketsecurity.core.classes import Diff +from socketsecurity.core.messages import Messages + + +@dataclass(frozen=True) +class ReportReferences: + full_scan_id: str + report_url: Optional[str] + diff_url: Optional[str] + is_full_scan: bool + + def console_link(self) -> Optional[tuple[str, str]]: + if self.is_full_scan: + if self.report_url: + return "Report Url", self.report_url + if self.diff_url: + return "Report Url", self.diff_url + else: + if self.diff_url: + return "Diff Url", self.diff_url + if self.report_url: + return "Report Url", self.report_url + return None + + +def build_report_references(diff_report: Diff) -> ReportReferences: + return ReportReferences( + full_scan_id=getattr(diff_report, "id", ""), + report_url=getattr(diff_report, "report_url", None), + diff_url=getattr(diff_report, "diff_url", ""), + is_full_scan=getattr(diff_report, "is_full_scan", False), + ) + + +def build_console_summary(diff_report: Diff, config: CliConfig) -> str: + references = build_report_references(diff_report) + is_full_scan = references.is_full_scan + link = references.console_link() + + if is_full_scan and not getattr(diff_report, "alerts_fetched", False): + lines = ["Full scan completed. Findings were not fetched for console output."] + if link: + lines.append(f"{link[0]}: {link[1]}") + return "\n".join(lines) + + selected_alerts = select_diff_alerts( + diff_report, + strict_blocking=config.strict_blocking, + ) + has_new_alerts = len(selected_alerts) > 0 + has_unchanged_alerts = ( + config.strict_blocking + and hasattr(diff_report, "unchanged_alerts") + and len(diff_report.unchanged_alerts) > 0 + ) + + if not has_new_alerts and not has_unchanged_alerts: + if link: + return f"No issues found\n{link[0]}: {link[1]}" + return "No issues found" + + new_blocking = sum(1 for issue in diff_report.new_alerts if issue.error) + new_warning = sum(1 for issue in diff_report.new_alerts if issue.warn) + + unchanged_blocking = 0 + unchanged_warning = 0 + if has_unchanged_alerts: + unchanged_blocking = sum( + 1 for issue in diff_report.unchanged_alerts if issue.error + ) + unchanged_warning = sum( + 1 for issue in diff_report.unchanged_alerts if issue.warn + ) + + selected_diff = clone_diff_with_selected_alerts(diff_report, selected_alerts) + console_security_comment = Messages.create_console_security_alert_table( + selected_diff + ) + + blocking_label = "Blocking issues" if is_full_scan else "NEW blocking issues" + warning_label = "Warning issues" if is_full_scan else "NEW warning issues" + + lines = ["Security issues detected by Socket Security:"] + if new_blocking > 0: + lines.append(f" - {blocking_label}: {new_blocking}") + if new_warning > 0: + lines.append(f" - {warning_label}: {new_warning}") + if unchanged_blocking > 0: + lines.append( + f" - EXISTING blocking issues: {unchanged_blocking} " + "(causing failure due to --strict-blocking)" + ) + if unchanged_warning > 0: + lines.append(f" - EXISTING warning issues: {unchanged_warning}") + if is_full_scan: + lines.append( + " Reported against the whole repository, with no baseline to compare " + "against, so these do not affect the exit code." + ) + + if link: + lines.append(f"{link[0]}: {link[1]}") + lines.append("") + lines.append(str(console_security_comment)) + return "\n".join(lines) + + +def build_socket_json_report( + diff_report: Diff, + config: CliConfig, +) -> dict[str, object]: + references = build_report_references(diff_report) + selected_alerts = select_diff_alerts( + diff_report, + strict_blocking=config.strict_blocking, + ) + selected_diff = clone_diff_with_selected_alerts(diff_report, selected_alerts) + report = Messages.create_security_comment_json(selected_diff) + legal_flag = getattr(config, "legal", False) + repo = getattr(config, "repo", None) + branch = getattr(config, "branch", None) + commit_sha = getattr(config, "commit_sha", None) + report["full_scan_id"] = references.full_scan_id + report["diff_url"] = references.diff_url + report["report_url"] = references.report_url + report["repo"] = repo if isinstance(repo, str) or repo is None else None + report["branch"] = branch if isinstance(branch, str) or branch is None else None + report["commit_sha"] = ( + commit_sha if isinstance(commit_sha, str) or commit_sha is None else None + ) + report["legal_mode"] = legal_flag if isinstance(legal_flag, bool) else False + return report diff --git a/socketsecurity/output.py b/socketsecurity/output.py index 566e4d3..3e88f89 100644 --- a/socketsecurity/output.py +++ b/socketsecurity/output.py @@ -16,6 +16,7 @@ from .config import CliConfig from .core.classes import Diff, Issue from .core.messages import Messages +from .core.report_output import build_console_summary, build_socket_json_report from .fossa_compat import build_fossa_report_payload @@ -229,88 +230,13 @@ def save_sbom_file(self, diff_report: Diff, sbom_file_name: Optional[str] = None def build_summary_text(self, diff_report: Diff) -> str: """Render the console summary text for stdout and file output.""" - is_full_scan = getattr(diff_report, "is_full_scan", False) - if is_full_scan and not getattr(diff_report, "alerts_fetched", False): - lines = ["Full scan completed. Findings were not fetched for console output."] - report_link = getattr(diff_report, "report_url", "") or getattr( - diff_report, "diff_url", "" - ) - if report_link: - lines.append(f"Report Url: {report_link}") - return "\n".join(lines) - - selected_alerts = select_diff_alerts(diff_report, strict_blocking=self.config.strict_blocking) - has_new_alerts = len(selected_alerts) > 0 - has_unchanged_alerts = ( - self.config.strict_blocking and - hasattr(diff_report, 'unchanged_alerts') and - len(diff_report.unchanged_alerts) > 0 - ) - - if not has_new_alerts and not has_unchanged_alerts: - return "No issues found" - - new_blocking = sum(1 for issue in diff_report.new_alerts if issue.error) - new_warning = sum(1 for issue in diff_report.new_alerts if issue.warn) - - unchanged_blocking = 0 - unchanged_warning = 0 - if has_unchanged_alerts: - unchanged_blocking = sum(1 for issue in diff_report.unchanged_alerts if issue.error) - unchanged_warning = sum(1 for issue in diff_report.unchanged_alerts if issue.warn) - - selected_diff = clone_diff_with_selected_alerts(diff_report, selected_alerts) - console_security_comment = Messages.create_console_security_alert_table(selected_diff) - - # A full scan has no baseline, so everything it carries is a finding in the - # repository rather than one the change introduced. Calling those NEW would - # also contradict the exit code, which stays 0 because no finding can be - # attributed to the change. - blocking_label = "Blocking issues" if is_full_scan else "NEW blocking issues" - warning_label = "Warning issues" if is_full_scan else "NEW warning issues" - link_label = "Report Url" if is_full_scan else "Diff Url" - - lines = ["Security issues detected by Socket Security:"] - if new_blocking > 0: - lines.append(f" - {blocking_label}: {new_blocking}") - if new_warning > 0: - lines.append(f" - {warning_label}: {new_warning}") - if unchanged_blocking > 0: - lines.append( - f" - EXISTING blocking issues: {unchanged_blocking} (causing failure due to --strict-blocking)" - ) - if unchanged_warning > 0: - lines.append(f" - EXISTING warning issues: {unchanged_warning}") - if is_full_scan: - lines.append( - " Reported against the whole repository, with no baseline to compare " - "against, so these do not affect the exit code." - ) - - report_link = getattr(diff_report, "report_url", "") or getattr(diff_report, "diff_url", "") - lines.append(f"{link_label}: {report_link}") - lines.append("") - lines.append(str(console_security_comment)) - return "\n".join(lines) + return build_console_summary(diff_report, self.config) def build_json_report(self, diff_report: Diff) -> dict: """Build the JSON report payload for stdout and file output.""" if getattr(self.config, "legal_format", "socket") == "fossa": return build_fossa_report_payload(diff_report, self.config) - - selected_alerts = select_diff_alerts(diff_report, strict_blocking=self.config.strict_blocking) - selected_diff = clone_diff_with_selected_alerts(diff_report, selected_alerts) - report = Messages.create_security_comment_json(selected_diff) - legal_flag = getattr(self.config, "legal", False) - repo = getattr(self.config, "repo", None) - branch = getattr(self.config, "branch", None) - commit_sha = getattr(self.config, "commit_sha", None) - report["report_url"] = getattr(diff_report, "report_url", None) - report["repo"] = repo if isinstance(repo, str) or repo is None else None - report["branch"] = branch if isinstance(branch, str) or branch is None else None - report["commit_sha"] = commit_sha if isinstance(commit_sha, str) or commit_sha is None else None - report["legal_mode"] = legal_flag if isinstance(legal_flag, bool) else False - return report + return build_socket_json_report(diff_report, self.config) def save_json_file(self, diff_report: Diff, json_file_name: Optional[str] = None) -> None: if not json_file_name: diff --git a/tests/unit/test_report_output.py b/tests/unit/test_report_output.py new file mode 100644 index 0000000..6e3db22 --- /dev/null +++ b/tests/unit/test_report_output.py @@ -0,0 +1,271 @@ +import copy +import json + +from socketsecurity.config import CliConfig +from socketsecurity.core.classes import Diff, Issue +from socketsecurity.core.report_output import ( + build_console_summary, + build_report_references, + build_socket_json_report, +) + + +def _config(*, strict_blocking: bool = False) -> CliConfig: + return CliConfig( + api_token="", + repo="fictional-repo", + strict_blocking=strict_blocking, + ) + + +def _issue( + title: str, + *, + error: bool = False, + warn: bool = False, +) -> Issue: + return Issue( + pkg_type="pypi", + pkg_name="fictional-package", + pkg_version="1.2.3", + category="supply-chain", + type=title, + severity="high" if error else "medium", + pkg_id="pypi/fictional-package@1.2.3", + props={}, + key=title, + error=error, + warn=warn, + ignore=False, + monitor=False, + description=f"Description for {title}", + title=title, + emoji="!", + next_step_title="Review the package", + suggestion="Use a safe version", + introduced_by=[("fictional-root", "requirements.txt")], + manifests="requirements.txt", + url="https://socket.dev/pypi/package/fictional-package/alerts/1.2.3", + purl="pkg:pypi/fictional-package@1.2.3", + ) + + +def _diff( + *, + new_alerts: list[Issue] | None = None, + unchanged_alerts: list[Issue] | None = None, + report_url: str | None = "https://socket.dev/orgs/fictional/reports/full-123", + diff_url: str | None = "https://socket.dev/orgs/fictional/diffs/diff-456", + is_full_scan: bool = False, + alerts_fetched: bool = True, +) -> Diff: + return Diff( + id="full-123", + new_scan_id="full-123", + report_url=report_url, + diff_url=diff_url, + is_full_scan=is_full_scan, + alerts_fetched=alerts_fetched, + new_alerts=list(new_alerts or []), + unchanged_alerts=list(unchanged_alerts or []), + ) + + +def test_comparison_prefers_diff_url_and_json_preserves_both_references(): + diff = _diff( + new_alerts=[_issue("New warning", warn=True)], + report_url="https://reports.example.test/full?scan=one", + diff_url="https://compare.example.test/pairs/a%2Fb?view=full", + ) + original = copy.deepcopy(diff.__dict__) + + summary = build_console_summary(diff, _config()) + report = build_socket_json_report(diff, _config()) + + assert "Diff Url: https://compare.example.test/pairs/a%2Fb?view=full" in summary + assert "Report Url:" not in summary + assert report["full_scan_id"] == "full-123" + assert report["diff_url"] == "https://compare.example.test/pairs/a%2Fb?view=full" + assert report["report_url"] == "https://reports.example.test/full?scan=one" + assert diff.__dict__ == original + + +def test_clean_comparison_keeps_message_first_and_includes_diff_link(): + diff = _diff(new_alerts=[]) + + summary = build_console_summary(diff, _config()) + report = build_socket_json_report(diff, _config()) + + assert summary.splitlines()[0] == "No issues found" + assert "Diff Url: https://socket.dev/orgs/fictional/diffs/diff-456" in summary + assert report["new_alerts"] == [] + assert report["scan_failed"] is False + + +def test_warning_and_blocking_summary_counts_and_json_failure_are_preserved(): + diff = _diff( + new_alerts=[ + _issue("Blocking alert", error=True), + _issue("Warning alert", warn=True), + ] + ) + + summary = build_console_summary(diff, _config()) + report = build_socket_json_report(diff, _config()) + decoded_alerts = [json.loads(json.dumps(alert)) for alert in report["new_alerts"]] + + assert "NEW blocking issues: 1" in summary + assert "NEW warning issues: 1" in summary + assert "Blocking alert" in summary + assert "Warning alert" in summary + assert report["scan_failed"] is True + assert [alert["title"] for alert in decoded_alerts] == [ + "Blocking alert", + "Warning alert", + ] + + +def test_strict_blocking_adds_unchanged_alerts_without_changing_references(): + unchanged = _issue("Existing blocking alert", error=True) + diff = _diff(new_alerts=[], unchanged_alerts=[unchanged]) + + ordinary = build_console_summary(diff, _config()) + strict = build_console_summary(diff, _config(strict_blocking=True)) + ordinary_json = build_socket_json_report(diff, _config()) + strict_json = build_socket_json_report(diff, _config(strict_blocking=True)) + + assert "Existing blocking alert" not in ordinary + assert "Existing blocking alert" in strict + assert ordinary_json["new_alerts"] == [] + assert [alert["title"] for alert in strict_json["new_alerts"]] == [ + "Existing blocking alert" + ] + assert ordinary_json["diff_url"] == strict_json["diff_url"] + assert ordinary_json["report_url"] == strict_json["report_url"] + + +def test_full_scan_uses_report_url_and_non_new_finding_labels(): + diff = _diff( + new_alerts=[_issue("Full scan warning", warn=True)], + is_full_scan=True, + ) + + summary = build_console_summary(diff, _config()) + + assert "Blocking issues:" not in summary + assert "Warning issues: 1" in summary + assert "NEW warning issues" not in summary + assert "whole repository, with no baseline" in summary + assert "Report Url: https://socket.dev/orgs/fictional/reports/full-123" in summary + assert "Diff Url:" not in summary + + +def test_clean_full_scan_uses_diff_url_as_report_url_fallback(): + diff = _diff( + new_alerts=[], + report_url="", + diff_url="https://reports.example.test/full-scan-fallback", + is_full_scan=True, + ) + + summary = build_console_summary(diff, _config()) + + assert summary.splitlines()[0] == "No issues found" + assert "Report Url: https://reports.example.test/full-scan-fallback" in summary + assert "Diff Url:" not in summary + + +def test_full_scan_without_fetched_findings_retains_explicit_state_and_report_link(): + diff = _diff(is_full_scan=True, alerts_fetched=False) + + summary = build_console_summary(diff, _config()) + + assert summary.startswith( + "Full scan completed. Findings were not fetched for console output." + ) + assert "No issues found" not in summary + assert "Report Url: https://socket.dev/orgs/fictional/reports/full-123" in summary + + +def test_comparison_report_url_fallback_and_missing_links(): + with_report = _diff( + new_alerts=[_issue("Warning", warn=True)], + diff_url="", + report_url="https://reports.example.test/alternate", + ) + without_urls = _diff( + new_alerts=[_issue("Warning", warn=True)], + diff_url="", + report_url="", + ) + + report_fallback_summary = build_console_summary(with_report, _config()) + no_link_summary = build_console_summary(without_urls, _config()) + + assert ( + "Report Url: https://reports.example.test/alternate" in report_fallback_summary + ) + assert "Diff Url:" not in report_fallback_summary + assert "Report Url:" not in no_link_summary + assert "Diff Url:" not in no_link_summary + assert "None" not in no_link_summary + assert "undefined" not in no_link_summary + + +def test_console_link_preserves_alternate_server_url_exactly(): + custom_url = "https://reports.example.test:9443/tenant/a%2Fb?view=x%2Fy&lang=en" + diff = _diff(report_url=custom_url, diff_url="") + + summary = build_console_summary(diff, _config()) + references = build_report_references(diff) + + assert summary.endswith(f"Report Url: {custom_url}") + assert references.console_link() == ("Report Url", custom_url) + + +def test_default_socket_json_key_set_and_metadata_remain_unchanged(): + diff = _diff(new_alerts=[]) + config = CliConfig( + api_token="", + repo="fictional-repo", + branch="feature/example", + commit_sha="abc123", + legal=True, + ) + + report = build_socket_json_report(diff, config) + + assert set(report) == { + "scan_failed", + "new_alerts", + "full_scan_id", + "diff_url", + "report_url", + "repo", + "branch", + "commit_sha", + "legal_mode", + } + assert report == { + "scan_failed": False, + "new_alerts": [], + "full_scan_id": "full-123", + "diff_url": "https://socket.dev/orgs/fictional/diffs/diff-456", + "report_url": "https://socket.dev/orgs/fictional/reports/full-123", + "repo": "fictional-repo", + "branch": "feature/example", + "commit_sha": "abc123", + "legal_mode": True, + } + + +def test_missing_reference_fields_keep_existing_json_defaults(): + diff = Diff(new_alerts=[]) + + summary = build_console_summary(diff, _config()) + report = build_socket_json_report(diff, _config()) + + assert summary == "No issues found" + assert report["full_scan_id"] == "" + assert report["diff_url"] == "" + assert report["report_url"] is None diff --git a/uv.lock b/uv.lock index 6df942c..7aeecc9 100644 --- a/uv.lock +++ b/uv.lock @@ -1292,7 +1292,7 @@ wheels = [ [[package]] name = "socketsecurity" -version = "2.10.11" +version = "2.10.12" source = { editable = "." } dependencies = [ { name = "beautifulsoup4" },