fix(comments): parse legacy alert rows defensively

Each row of the legacy comment table was unpacked through four consecutive
splits with no bounds checks: five cells, then the markdown link, then the
ecosystem, then the version. The row comes back from the provider's API, so a
cell carrying an extra "|", a package cell that is not a link, or a name with no
version raised out of the comment rewrite and ended the run before it reported
status. A scoped package name in Socket's own table reached the same place with
nobody doing anything unusual.

parse_alert_table_row returns None instead of raising for any row it cannot
read, and an unreadable row keeps its alert reported -- the safe direction, since
a row that cannot be parsed cannot be evaluated against the ignore commands
either.

Also pins change-type preservation against the real artifact conversion rather
than a stubbed field. The existing test assigned diffType by hand, so it would
have passed whether or not the conversion populated it; the new one runs real
DiffArtifact objects through both response shapes, and fails if the field is
dropped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: lelia

CHANGELOG.md

@@ -75,10 +75,13 @@
 - Shared security comment copy no longer describes GitLab merge request output
   as Socket for GitHub.
 - Updating a security comment in the legacy table format no longer raises on a
-  scoped package name. That path split the package cell on every `@`, so a name
-  carrying its own `@` unpacked into three values; it now splits from the right,
-  matching the current comment format. Ignore commands for a scoped package are
-  accepted there in both the ecosystem-qualified and bare forms.
+  malformed row. Each row was unpacked through four consecutive splits with no
+  bounds checks, so a cell carrying an extra `|`, a package cell that is not a
+  markdown link, or a name with no version ended the run before it reported
+  status — and a scoped package name in Socket's own table was enough to trigger
+  it. Rows are now parsed defensively, and a row that cannot be read keeps its
+  alert reported. Ignore commands for a scoped package are accepted there in both
+  the ecosystem-qualified and bare forms.
 - Server URLs read from `GITHUB_SERVER_URL` and `CI_SERVER_URL` are validated as
   http(s) URLs before being composed into a diff scan's external link, matching
   the check already applied to the other repository URLs read from CI.

socketsecurity/core/scm_comments.py

@@ -145,6 +145,35 @@ def process_security_comment(comment: Comment, comments) -> str:
 
         return new_body
 
+    @staticmethod
+    def parse_alert_table_row(line: str) -> Optional[tuple[str, str, str]]:
+        """Pull ``(ecosystem, package, version)`` out of a legacy alert table row.
+
+        Returns None for any row that does not have the expected shape rather than
+        raising. The row comes back from the provider's API, so its contents are
+        outside this process's control: a cell carrying an extra ``|``, a package
+        cell that is not a markdown link, or a name with no version all used to
+        raise out of the comment rewrite and take the run down before it reported
+        status. A row that cannot be read is a row whose alert stays reported.
+        """
+        cells = line.strip().lstrip("|").rstrip("|").split("|")
+        if len(cells) != 5:
+            return None
+        package = cells[1]
+        if "](" not in package:
+            return None
+        details = package.split("](", 1)[0].lstrip("[")
+        if "/" not in details:
+            return None
+        ecosystem, remainder = details.split("/", 1)
+        if "@" not in remainder:
+            return None
+        # Split from the right: a scoped name carries its own "@".
+        pkg_name, pkg_version = remainder.rsplit("@", 1)
+        if not pkg_name or not pkg_version:
+            return None
+        return ecosystem, pkg_name, pkg_version
+
     @staticmethod
     def process_original_security_comment(
             comment: Comment,
@@ -160,19 +189,21 @@ def process_original_security_comment(
                 start = True
                 lines.append(line)
             elif start and "end-socket-alerts-table" not in line and not Comments.is_heading_line(line) and line != '':
-                title, package, introduced_by, manifest, ci = line.lstrip("|").rstrip("|").split("|")
-                details, _ = package.split("](")
-                ecosystem, details = details.split("/", 1)
-                ecosystem = ecosystem.lstrip("[")
-                # Split from the right: a scoped name carries its own "@".
-                pkg_name, pkg_version = details.rsplit("@", 1)
+                parsed = Comments.parse_alert_table_row(line)
                 # 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, ecosystem)
-                    for name, version in ignore_commands
-                )
+                if parsed is None:
+                    # An unparseable row cannot be evaluated against the ignore
+                    # commands, so keep it: leaving an alert reported is the safe
+                    # direction, and the comment body is not ours to discard.
+                    ignore = ignore_all
+                else:
+                    ecosystem, pkg_name, pkg_version = parsed
+                    ignore = ignore_all or any(
+                        Comments.is_ignore(pkg_name, pkg_version, name, version, ecosystem)
+                        for name, version in ignore_commands
+                    )
                 if not ignore:
                     kept_alert = True
                     lines.append(line)

tests/core/test_diff_generation.py

@@ -1,8 +1,9 @@
 import json
-from dataclasses import fields
+from dataclasses import asdict, fields
 from pathlib import Path
 
 import pytest
+from socketdev.fullscans import DiffArtifact
 
 from socketsecurity.core import Core
 from socketsecurity.core.classes import Package
@@ -108,6 +109,39 @@ def test_create_diff_report_preserves_package_change_types(core, diff_input):
     assert diff.removed_packages == []
     assert {package.id for package in diff.replaced_packages} == {"dp2"}
 
+
+def _diff_artifact(change_type: str, flattened: bool) -> dict:
+    """A DiffArtifact of the given change type, in one of the two response shapes.
+
+    The API sends flattened artifacts; the older shape carries the dependency
+    context in a head/base ref instead, and Package.from_diff_artifact reads
+    diffType differently in each.
+    """
+    raw = json.loads(
+        (Path(__file__).parent.parent / "data/fullscans/diff/stream_diff.json").read_text()
+    )["data"]["artifacts"]["added"][0]
+    artifact = dict(raw, diffType=change_type, head=None, base=None)
+    if not flattened:
+        link = {"topLevelAncestors": ["x"], "direct": True, "artifact": None,
+                "dependencies": [], "manifestFiles": []}
+        key = "head" if change_type in ("added", "updated") else "base"
+        artifact[key] = [link]
+    return asdict(DiffArtifact.from_dict(artifact))
+
+
+@pytest.mark.parametrize("flattened", [True, False], ids=["flattened", "ref-shaped"])
+@pytest.mark.parametrize("change_type", ["added", "updated", "removed", "replaced"])
+def test_change_type_survives_artifact_conversion(change_type, flattened):
+    """The classification reads Package.diffType, so the conversion must set it.
+
+    create_diff_report buckets on this field alone. A conversion that dropped it
+    would silently report every update as an addition and every replacement as a
+    removal, which is the inaccuracy the change-type split exists to prevent.
+    """
+    package = Package.from_diff_artifact(_diff_artifact(change_type, flattened))
+
+    assert package.diffType == change_type
+
 def create_input(core):
     # Get two different scans to compare
     head_scan = core.get_full_scan("head")