fix(comments): close ignore authorization gaps

Author: lelia

CHANGELOG.md

@@ -58,6 +58,9 @@
 - On GitHub this is read from the effective repository permission and cached per
   commenter for the run. Write, maintain, or admin access is required; relationship
   labels such as `MEMBER` and `COLLABORATOR` are not treated as permissions.
+- A 404 from GitHub's collaborator-permission endpoint is treated as a definitive
+  denial rather than an unreadable permission, so the default `enforce` policy does
+  not honor ignore commands from users outside the repository.
 - GitLab notes carry no equivalent field, so project membership is read once per
   run (only when an ignore command is present) and Developer or above is required.
   If that lookup cannot be answered — a `CI_JOB_TOKEN` generally cannot read the
@@ -92,6 +95,8 @@
   accept scoped packages while remaining compatible with older bare-name replies.
   A leading npm scope is no longer mistaken for an ecosystem, so
   `ignore @types/node@*` no longer also ignores the package named `node`.
+- Ignore telemetry uses the same package matcher as alert suppression, so legacy
+  bare-name commands generate an event for the alert they suppress.
 - Dependency overviews preserve added, updated, removed, and replaced package
   classifications instead of presenting updates as new dependencies. Added and
   updated rows keep their diff badge; removed and replaced, which have no

socketsecurity/core/scm/github.py

@@ -8,6 +8,7 @@
 from socketsecurity import USER_AGENT
 from socketsecurity.core import log
 from socketsecurity.core.classes import Comment
+from socketsecurity.core.exceptions import APIFailure
 from socketsecurity.core.git_remote import parse_git_remote
 from socketsecurity.core.scm_comments import Comments
 from socketsecurity.socketcli import CliClient
@@ -273,6 +274,17 @@ def is_ignore_authorized(self, comment: Comment) -> bool:
                     permission = (
                         result["permission"].casefold() in self.WRITE_PERMISSIONS
                     )
+            except APIFailure as error:
+                if getattr(error, "status_code", None) == 404:
+                    # The repository was readable when its comments were listed,
+                    # so a missing collaborator permission is a definitive denial.
+                    permission = False
+                else:
+                    log.warning(
+                        "Could not read GitHub repository permission for "
+                        f"{author}: {error}"
+                    )
+                    permission = None
             except Exception as error:
                 log.warning(
                     f"Could not read GitHub repository permission for {author}: {error}"

socketsecurity/socketcli.py

@@ -138,6 +138,30 @@ def should_write_comment(disabled: bool, has_findings: bool, update_existing: bo
         return update_existing
     return True
 
+
+def _match_ignored_alerts_for_telemetry(
+    ignored_alerts: list,
+    ignore_all: bool,
+    ignore_commands: list[tuple[str, str]],
+) -> list:
+    """Return alerts attributable to one ignore comment."""
+    if ignore_all:
+        return list(ignored_alerts)
+    return [
+        alert
+        for alert in ignored_alerts
+        if any(
+            Comments.is_ignore(
+                alert.pkg_name,
+                alert.pkg_version,
+                name,
+                version,
+                alert.pkg_type,
+            )
+            for name, version in ignore_commands
+        )
+    ]
+
 def _select_pull_request_provider(integration_type: str, scm_type: str) -> str:
     """Prefer an active comment adapter when resolving pull request context."""
     return scm_type if scm_type in ("github", "gitlab") else integration_type
@@ -837,16 +861,11 @@ def _is_unprocessed(c):
                                 sender_id = str(user.get("id", ""))
 
                                 # Match this comment's targets to the actual ignored alerts
-                                matched_alerts = []
-                                if c_ignore_all:
-                                    matched_alerts = ignored_alerts
-                                else:
-                                    for alert in ignored_alerts:
-                                        full_name = f"{alert.pkg_type}/{alert.pkg_name}"
-                                        purl = (full_name, alert.pkg_version)
-                                        purl_star = (full_name, "*")
-                                        if purl in c_ignore_commands or purl_star in c_ignore_commands:
-                                            matched_alerts.append(alert)
+                                matched_alerts = _match_ignored_alerts_for_telemetry(
+                                    ignored_alerts,
+                                    c_ignore_all,
+                                    c_ignore_commands,
+                                )
 
                                 shared_fields = {
                                     "event_kind": "user-action",

tests/unit/test_git_remote.py

@@ -25,7 +25,7 @@
             ("gitlab.example.com", "acme/platform/widgets"),
         ),
         ("git://github.com/acme/widgets.git", ("github.com", "acme/widgets")),
-        # Cosmetic variation callers should not have to normalise themselves.
+        # Cosmetic variation callers should not have to normalize themselves.
         ("  https://github.com/acme/widgets/  ", ("github.com", "acme/widgets")),
         # Credentials in the URL must not leak into the host.
         ("https://user@github.com/acme/widgets", ("github.com", "acme/widgets")),

tests/unit/test_ignore_authorization.py

@@ -10,6 +10,7 @@
 import pytest
 
 from socketsecurity.core.classes import Comment
+from socketsecurity.core.exceptions import APIFailure
 from socketsecurity.core.scm.github import Github
 from socketsecurity.core.scm.gitlab import Gitlab
 from socketsecurity.core.scm_comments import Comments
@@ -69,6 +70,17 @@ def test_github_permission_is_fetched_once_per_commenter():
     assert github.calls == ["repos/o/r/collaborators/maintainer/permission"]
 
 
+def test_github_404_is_a_cached_unauthorized_result():
+    github = _github(
+        raises=APIFailure("not a collaborator", status_code=404),
+    )
+    comment = _comment(user={"login": "outsider"})
+
+    assert github.is_ignore_authorized(comment) is False
+    assert github.is_ignore_authorized(comment) is False
+    assert github.calls == ["repos/o/r/collaborators/outsider/permission"]
+
+
 def test_github_unreadable_permission_honors_the_command_with_a_warning(caplog):
     github = _github(raises=Exception("403 Forbidden"))
 

tests/unit/test_ignore_telemetry_filtering.py

@@ -1,9 +1,11 @@
 """Tests for the +1 reaction dedup logic used to filter ignore comments for telemetry."""
 
+from types import SimpleNamespace
 from unittest.mock import Mock
 
 from socketsecurity.core.classes import Comment
 from socketsecurity.core.scm_comments import Comments
+from socketsecurity.socketcli import _match_ignored_alerts_for_telemetry
 
 
 def _make_comment(body: str, thumbs_up: int = 0, comment_id: int = 1, user: dict | None = None) -> Comment:
@@ -161,6 +163,22 @@ def test_no_unprocessed_means_no_telemetry(self):
         assert len(unprocessed) == 0
 
 
+def test_push_telemetry_matches_a_legacy_bare_name_ignore():
+    comment = _make_comment("SocketSecurity ignore lodash@4.17.21")
+    _, ignore_commands = Comments.get_ignore_options({"ignore": [comment]})
+    alert = SimpleNamespace(
+        pkg_type="npm",
+        pkg_name="lodash",
+        pkg_version="4.17.21",
+    )
+
+    assert _match_ignored_alerts_for_telemetry(
+        [alert],
+        False,
+        ignore_commands,
+    ) == [alert]
+
+
 def _build_event(comment, ignore_all=False, ignore_commands=None, artifact_input=None, artifact_purl=None):
     """Mirrors the event construction logic in socketcli.py."""
     from datetime import datetime, timezone