Fix orphaned tags and empty tables in PR comments

A whitespace-only line closes a CommonMark HTML block. Optional sections that
rendered as empty left one behind inside the alerts table, so the indented
closing tags after it were rendered as a literal code block reading
`</blockquote></details>` instead of markup.

- Drop blank lines from generated comment markup and keep indentation below the
  four spaces that start a code block.
- Collapse alert descriptions, suggestions and license findings onto a single
  line so multi-line API text cannot break the table either.
- Replace the comment body with a short confirmation when no alerts are left to
  report, instead of keeping the caution banner above a table with no rows. The
  comment marker is preserved so the same comment is updated later.
- Apply ignore-all to the pre-2.0.55 Markdown table format. The check was made
  once per ignore command and an ignore-all comment produces none, so no rows
  were removed.

Bumps to 2.6.8.

Author: lelia

CHANGELOG.md

@@ -1,5 +1,28 @@
 # Changelog
 
+## 2.6.8
+
+### Fixed: pull request comments no longer show orphaned tags or an empty table
+
+- Optional sections that rendered as empty, such as the ignore instructions
+  suppressed by `--disable-ignore`, left a whitespace-only line in the alerts
+  table. That line closed the surrounding HTML block, and the indented
+  `</blockquote></details>` tags after it were rendered as a literal code block.
+  Generated comment markup now omits blank lines and stays under the indentation
+  that starts a code block.
+- Alert descriptions, suggestions and license findings are collapsed onto a
+  single line so multi-line API text cannot break the table markup either.
+- When a pull request has no alerts left to report, the security comment is
+  replaced with a short confirmation instead of keeping the "Caution" banner
+  above a table with no rows. This happens both when a later commit resolves
+  every alert and when every alert is ignored by comment. The comment marker is
+  preserved, so a commit that reintroduces an alert updates the same comment
+  rather than posting a second one.
+- `@SocketSecurity ignore-all` now applies to comments written by CLI versions
+  before 2.0.55, which use the older Markdown alerts table. The check was made
+  once per ignore command, and an ignore-all comment produces none, so no rows
+  were removed.
+
 ## 2.6.7
 
 ### Changed: bump pinned @coana-tech/cli to 15.10.23

pyproject.toml

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

socketsecurity/__init__.py

@@ -1,3 +1,3 @@
 __author__ = 'socket.dev'
-__version__ = '2.6.7'
+__version__ = '2.6.8'
 USER_AGENT = f'SocketPythonCLI/{__version__}'

socketsecurity/core/messages.py

@@ -806,6 +806,90 @@ def create_security_comment_gitlab(diff: Diff) -> dict:
 
         return gitlab_report
 
+    # A blank line terminates a CommonMark HTML block. When that happens inside
+    # the alerts table the closing tags that follow are no longer treated as
+    # markup, and because they are indented four or more spaces they render as a
+    # literal code block containing `</blockquote></details>` instead.
+    MAX_HTML_INDENT = 3
+
+    @staticmethod
+    def inline_html_text(value) -> str:
+        """
+        Collapses API supplied text onto a single line.
+
+        Alert descriptions and suggestions are interpolated into the comment HTML,
+        so an embedded newline would otherwise be able to close the surrounding
+        HTML block early.
+
+        :param value: The value to flatten. ``None`` becomes an empty string.
+        :return: str - The value with all whitespace runs collapsed to a single space.
+        """
+        if value is None:
+            return ""
+        return " ".join(str(value).split())
+
+    @staticmethod
+    def normalize_comment_html(comment: str) -> str:
+        """
+        Makes generated comment markup safe for the CommonMark renderers used by
+        GitHub and GitLab.
+
+        Drops whitespace-only lines (an optional section that rendered as empty
+        leaves one behind) and caps indentation below the four spaces that would
+        start an indented code block. Intentional separators - lines that are
+        genuinely empty - are preserved so markdown blocks still break apart.
+
+        :param comment: str - The generated comment body.
+        :return: str - The comment body with unrenderable whitespace removed.
+        """
+        lines = []
+        for line in comment.split("\n"):
+            if line and not line.strip():
+                continue
+            stripped = line.lstrip()
+            indent = min(len(line) - len(stripped), Messages.MAX_HTML_INDENT)
+            lines.append(" " * indent + stripped)
+        return "\n".join(lines)
+
+    @staticmethod
+    def security_comment_no_alerts_template(view_report_url: str = "") -> str:
+        """
+        Generates the body used when there is nothing left to report.
+
+        Alerts raised on an early commit are frequently resolved later in the same
+        pull request. Rewriting the comment to this body keeps the Socket comment
+        in place - so a later commit that reintroduces an alert updates it rather
+        than posting a second comment - without leaving the "Caution" banner above
+        an empty alerts table.
+
+        :param view_report_url: str - Optional link to the full Socket report.
+        :return: str - The formatted Markdown/HTML string.
+        """
+        lines = [
+            "<!-- socket-security-comment-actions -->",
+            "",
+            "> **✅ Socket Security**  ",
+            "> No dependency alerts to report. Any alerts previously reported on this "
+            "pull request have been resolved or ignored.",
+        ]
+        if view_report_url:
+            lines += ["", f"[View full report]({view_report_url})"]
+        return "\n".join(lines) + "\n"
+
+    @staticmethod
+    def get_view_report_url(diff: Diff) -> str:
+        """
+        Resolves the report link for a diff, preferring the PR/MR diff view.
+
+        :param diff: Diff - Diff report to pull the URL from.
+        :return: str - The report URL, or an empty string when neither is set.
+        """
+        if getattr(diff, "diff_url", None):
+            return diff.diff_url
+        if getattr(diff, "report_url", None):
+            return diff.report_url
+        return ""
+
     @staticmethod
     def security_comment_template(diff: Diff, config=None) -> str:
         """
@@ -819,7 +903,7 @@ def security_comment_template(diff: Diff, config=None) -> str:
         # Group license policy violations by PURL (ecosystem/package@version)
         license_groups = {}
         security_alerts = []
-        
+
         for alert in diff.new_alerts:
             if alert.type == "licenseSpdxDisj":
                 purl_key = f"{alert.pkg_type}/{alert.pkg_name}@{alert.pkg_version}"
@@ -829,6 +913,13 @@ def security_comment_template(diff: Diff, config=None) -> str:
             else:
                 security_alerts.append(alert)
 
+        view_report_url = Messages.get_view_report_url(diff)
+
+        # Without this the caution banner would sit above a table with no rows,
+        # which is how a comment looks once every alert it raised is resolved.
+        if not security_alerts and not license_groups:
+            return Messages.security_comment_no_alerts_template(view_report_url)
+
         # Start of the comment
         comment = """<!-- socket-security-comment-actions -->
 
@@ -875,15 +966,15 @@ def security_comment_template(diff: Diff, config=None) -> str:
   </td>
   <td>
     <details {details_open}>
-      <summary>{alert.pkg_name}@{alert.pkg_version} - {alert.title}</summary>
-      <p><strong>Note:</strong> {alert.description}</p>
+      <summary>{alert.pkg_name}@{alert.pkg_version} - {Messages.inline_html_text(alert.title)}</summary>
+      <p><strong>Note:</strong> {Messages.inline_html_text(alert.description)}</p>
       <p><strong>Source:</strong> <a href="{manifest_url}">Manifest File</a></p>
       <p>ℹ️ Read more on:
       <a href="{alert.purl}">This package</a> |
       <a href="{alert.url}">This alert</a> |
       <a href="https://socket.dev/alerts/malware">What is known malware?</a></p>
       <blockquote>
-        <p><em>Suggestion:</em> {alert.suggestion}</p>
+        <p><em>Suggestion:</em> {Messages.inline_html_text(alert.suggestion)}</p>
         {ignore_html}
       </blockquote>
     </details>
@@ -917,7 +1008,7 @@ def security_comment_template(diff: Diff, config=None) -> str:
       <ul>
 """
             for finding in license_findings:
-                comment += f"        <li>{finding}</li>\n"
+                comment += f"        <li>{Messages.inline_html_text(finding)}</li>\n"
 
 
             # Generate proper manifest URL for license violations
@@ -944,13 +1035,6 @@ def security_comment_template(diff: Diff, config=None) -> str:
     """
 
         # Close table
-        # Use diff_url for PRs, report_url for non-PR scans
-        view_report_url = ""
-        if hasattr(diff, 'diff_url') and diff.diff_url:
-            view_report_url = diff.diff_url
-        elif hasattr(diff, 'report_url') and diff.report_url:
-            view_report_url = diff.report_url
-            
         comment += f"""
   </tbody>
 </table>
@@ -959,7 +1043,7 @@ def security_comment_template(diff: Diff, config=None) -> str:
 [View full report]({view_report_url}?action=error%2Cwarn)
     """
 
-        return comment
+        return Messages.normalize_comment_html(comment)
 
     @staticmethod
     def get_severity_icon(severity: str) -> str:

socketsecurity/core/scm_comments.py

@@ -1,12 +1,16 @@
 import json
+import re
 
 from requests import Response
 
 from socketsecurity.core import log
 from socketsecurity.core.classes import Comment, Issue
+from socketsecurity.core.messages import Messages
 
 
 class Comments:
+    VIEW_REPORT_PATTERN = re.compile(r"\[View full report\]\(([^)\s]+)\)")
+
     @staticmethod
     def process_response(response: Response) -> dict:
         output = {}
@@ -84,6 +88,20 @@ def is_heading_line(line) -> bool:
             is_heading_line = False
         return is_heading_line
 
+    @staticmethod
+    def extract_report_url(body: str) -> str:
+        """
+        Pulls the Socket report link out of an existing comment body so it can be
+        carried over when the comment is rewritten.
+
+        :param body: str - The existing comment body.
+        :return: str - The report URL without its query string, or "" if absent.
+        """
+        match = Comments.VIEW_REPORT_PATTERN.search(body)
+        if not match:
+            return ""
+        return match.group(1).split("?", 1)[0]
+
     @staticmethod
     def process_security_comment(comment: Comment, comments) -> str:
         ignore_all, ignore_commands = Comments.get_ignore_options(comments)
@@ -102,6 +120,7 @@ def process_original_security_comment(
     ) -> str:
         start = False
         lines = []
+        kept_alert = False
         for line in comment.body_list:
             line = line.strip()
             if "start-socket-alerts-table" in line:
@@ -114,17 +133,25 @@ def process_original_security_comment(
                 ecosystem = ecosystem.lstrip("[")
                 pkg_name, pkg_version = details.split("@")
                 pkg_name = f"{ecosystem}/{pkg_name}"
-                ignore = False
-                for name, version in ignore_commands:
-                    if ignore_all or Comments.is_ignore(pkg_name, pkg_version, name, version):
-                        ignore = True
+                # ignore_all has to be checked outside the loop: an ignore-all
+                # comment produces no ignore_commands, so a loop-internal check
+                # never runs and every row was kept.
+                ignore = ignore_all or any(
+                    Comments.is_ignore(pkg_name, pkg_version, name, version)
+                    for name, version in ignore_commands
+                )
                 if not ignore:
+                    kept_alert = True
                     lines.append(line)
             elif "end-socket-alerts-table" in line:
                 start = False
                 lines.append(line)
             else:
                 lines.append(line)
+        if not kept_alert:
+            return Messages.security_comment_no_alerts_template(
+                Comments.extract_report_url("\n".join(comment.body_list))
+            )
         return "\n".join(lines)
 
     @staticmethod
@@ -145,17 +172,21 @@ def process_updated_security_comment(
         """
         lines = []
         ignore_section = False
+        kept_alert = False  # Whether any alert row survived the ignore commands
         pkg_name = pkg_version = ""  # Track current package and version
 
         # Loop through the comment lines
         for line in comment.body_list:
-            line = line.strip()
+            # Match on the stripped line but keep the original, so the markup is
+            # rewritten with the same indentation it was generated with.
+            line = line.rstrip("\r")
+            stripped = line.strip()
 
             # Detect the start of an alert section
-            if line.startswith("<!-- start-socket-alert-"):
+            if stripped.startswith("<!-- start-socket-alert-"):
                 # Extract package name and version from the comment
                 try:
-                    start_marker = line[len("<!-- start-socket-alert-"):-4]  # Strip the comment markers
+                    start_marker = stripped[len("<!-- start-socket-alert-"):-4]  # Strip the comment markers
                     pkg_name, pkg_version = start_marker.split("@")  # Extract pkg_name and pkg_version
                 except ValueError:
                     pkg_name, pkg_version = "", ""
@@ -168,10 +199,11 @@ def process_updated_security_comment(
 
                 # If not ignored, include this start marker
                 if not ignore_section:
+                    kept_alert = True
                     lines.append(line)
 
             # Detect the end of an alert section
-            elif line.startswith("<!-- end-socket-alert-"):
+            elif stripped.startswith("<!-- end-socket-alert-"):
                 # Only include if we are not ignoring this section
                 if not ignore_section:
                     lines.append(line)
@@ -181,7 +213,14 @@ def process_updated_security_comment(
             elif not ignore_section:
                 lines.append(line)
 
-        return "\n".join(lines)
+        # Every row was ignored, so drop the table rather than leaving the caution
+        # banner sitting above an empty one.
+        if not kept_alert:
+            return Messages.security_comment_no_alerts_template(
+                Comments.extract_report_url("\n".join(comment.body_list))
+            )
+
+        return Messages.normalize_comment_html("\n".join(lines))
 
     @staticmethod
     def extract_alert_details_from_row(row: str, ignore_all: bool, ignore_commands: list[tuple[str, str]]) -> tuple: