fix(review): address authorization and full-scan findings

Author: lelia

CHANGELOG.md

@@ -43,16 +43,21 @@
 - `--generate-license` and `--legal-format fossa` fetch the package list on this
   path, so attribution files generated from a branch pipeline are complete rather
   than empty.
+- Console-only full scans link to the Socket report and state that findings were
+  not fetched for console output instead of presenting an empty local alert list
+  as "No issues found."
+- License enrichment keeps the package namespace in PURL requests and response
+  matching, so scoped npm packages and namespaced Maven packages receive their
+  license details.
 
 ### Changed: `@SocketSecurity ignore` requires write access
 
 - An ignore command suppresses a security alert, but the CLI honored one from any
   commenter, including a drive-by comment from someone with no access to the
   repository. Commands are now accepted only from an author with write access.
-- On GitHub this is read from the `author_association` GitHub already returns with
-  each comment, so it costs no extra request: `OWNER`, `MEMBER` and `COLLABORATOR`
-  are honored, and `CONTRIBUTOR`, `FIRST_TIME_CONTRIBUTOR`, `MANNEQUIN` and `NONE`
-  are not.
+- 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.
 - 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

docs/cli-reference.md

@@ -701,11 +701,12 @@ reported. `--disable-ignore` turns the feature off entirely.
 
 | Provider | How access is determined | If it cannot be determined |
 |:---------|:-------------------------|:---------------------------|
-| GitHub | The `author_association` returned with each comment. `OWNER`, `MEMBER` and `COLLABORATOR` are honored. | Treated as unauthorized. |
+| GitHub | Effective repository permission, read once per commenter per run. Write, maintain, or admin access is honored. | The command is honored and a warning is logged. |
 | GitLab | Project membership, read once per run when an ignore command is present. Developer (30) or above is honored. | The command is honored and a warning is logged. |
 
-GitLab notes carry no permission field, so the check needs a `GITLAB_TOKEN` that
-can read `GET /projects/:id/members/all`. A `CI_JOB_TOKEN` generally cannot.
+The GitHub check needs a token that can read repository metadata. GitLab notes
+carry no permission field, so that check needs a `GITLAB_TOKEN` that can read
+`GET /projects/:id/members/all`. A `CI_JOB_TOKEN` generally cannot.
 
 `--ignore-authorization` decides what happens when access cannot be determined:
 

socketsecurity/core/__init__.py

@@ -1279,6 +1279,7 @@ def create_full_scan_with_report_url(
         diff.report_url = f"{base_socket}/{self.config.org_slug}/sbom/{new_full_scan.id}"
         diff.diff_url = diff.report_url
         diff.id = new_full_scan.id
+        diff.is_full_scan = True
 
         needs_alerts = (
             self.cli_config is not None
@@ -1324,6 +1325,7 @@ def create_full_scan_with_report_url(
                         if (alert.error or alert.warn) and alert_str not in consolidated:
                             diff.new_alerts.append(alert)
                             consolidated.add(alert_str)
+                diff.alerts_fetched = True
 
             sbom_end = time.time()
             log.info(
@@ -1344,11 +1346,16 @@ def _add_license_details(self, packages: dict[str, Package]) -> dict[str, Packag
         enrichment lands on the map the caller keeps.
         """
         batch_size = self.cli_config.max_purl_batch_size if self.cli_config else 5000
+        packages_by_purl = {}
+        for package in packages.values():
+            qualified_name = package.name
+            if package.namespace:
+                qualified_name = f"{package.namespace.strip('/')}/{qualified_name}"
+            packages_by_purl[
+                f"{package.type}/{qualified_name}@{package.version}"
+            ] = package
         self.get_license_text_via_purl(
-            {
-                f"{package.type}/{package.name}@{package.version}": package
-                for package in packages.values()
-            },
+            packages_by_purl,
             batch_size=batch_size,
         )
         return packages
@@ -1657,6 +1664,9 @@ def get_license_text_via_purl(self, packages: dict[str, Package], batch_size: in
             for result in results:
                 ecosystem = result["type"]
                 name = result["name"]
+                namespace = (result.get("namespace") or "").strip("/")
+                if namespace and not name.startswith(f"{namespace}/"):
+                    name = f"{namespace}/{name}"
                 package_version = result["version"]
                 licenseDetails = result.get("licenseDetails")
                 licenseAttrib = result.get("licenseAttrib")

socketsecurity/core/classes.py

@@ -520,6 +520,8 @@ class Diff:
     report_url: str
     diff_url: str
     new_scan_id: str
+    is_full_scan: bool
+    alerts_fetched: bool
 
     def __init__(self, **kwargs):
         if kwargs:
@@ -541,6 +543,10 @@ def __init__(self, **kwargs):
             self.removed_alerts = []
         if not hasattr(self, "new_capabilities"):
             self.new_capabilities = {}
+        if not hasattr(self, "is_full_scan"):
+            self.is_full_scan = False
+        if not hasattr(self, "alerts_fetched"):
+            self.alerts_fetched = False
 
     def __str__(self):
         return json.dumps(self.__dict__)

socketsecurity/core/messages.py

@@ -997,8 +997,6 @@ def security_comment_template(diff: Diff, config=None) -> str:
             # Generate proper manifest URL
             manifest_url = Messages.get_manifest_file_url(diff, alert.manifests, config)
             pkg_label = Messages.html_text(f"{alert.pkg_name}@{alert.pkg_version}")
-            # The marker is read back verbatim when the comment is rewritten, so it
-            # keeps the raw name and only loses the ability to close the comment.
             pkg_marker = Messages.comment_marker_text(f"{alert.pkg_name}@{alert.pkg_version}")
             # Generate a table row for each alert
             ignore_html = (
@@ -1045,8 +1043,6 @@ def security_comment_template(diff: Diff, config=None) -> str:
             license_label = Messages.html_text(
                 f"{first_alert.pkg_name}@{first_alert.pkg_version}"
             )
-            # The marker is read back verbatim when the comment is rewritten, so it
-            # keeps the raw name and only loses the ability to close the comment.
             license_marker = Messages.comment_marker_text(
                 f"{first_alert.pkg_name}@{first_alert.pkg_version}"
             )
@@ -1293,8 +1289,7 @@ def create_remove_line(diff: Diff, md: MdUtils) -> MdUtils:
 
     # Change types the shared badge host publishes an image for. Removed and
     # replaced have no artwork, so they fall back to a bold text label rather than
-    # rendering a broken image; added and updated keep the badge the overview
-    # comment has always used.
+    # rendering a broken image; added and updated render the available badges.
     DIFF_BADGES = {
         "Added": "diff-added.svg",
         "Updated": "diff-updated.svg",

socketsecurity/core/scm/github.py

@@ -154,6 +154,8 @@ def from_env(cls, pr_number: Optional[str] = None) -> 'GithubConfig':
 
 
 class Github:
+    WRITE_PERMISSIONS = frozenset({"write", "maintain", "admin"})
+
     def __init__(
         self,
         client: CliClient,
@@ -163,6 +165,10 @@ def __init__(
         self.config = config or GithubConfig.from_env()
         self.client = client
         self.ignore_authorization = ignore_authorization
+        # Permission is stable for the duration of one CLI run. Cache both
+        # positive and negative answers so several ignore comments by the same
+        # author do not each make an API request.
+        self._ignore_permission_cache: dict[str, Optional[bool]] = {}
 
         if not self.config.token:
             log.error("Unable to get Github API Token")
@@ -236,12 +242,59 @@ def get_comments_for_pr(self) -> dict:
     def is_ignore_authorized(self, comment: Comment) -> bool:
         """Whether a commenter may suppress alerts with @SocketSecurity ignore.
 
-        GitHub returns the author's relationship to the repository on every issue
-        comment, so this costs no extra request and is definitive. A missing value
-        is treated as unauthorized rather than trusted.
+        ``author_association`` describes a social relationship to the repository,
+        not the author's role: an organization member or outside collaborator can
+        still have read-only access. Ask GitHub for the effective repository
+        permission instead, and cache the answer for subsequent comments.
         """
-        association = (getattr(comment, "author_association", "") or "").upper()
-        return association in Comments.WRITE_ACCESS_ASSOCIATIONS
+        author = Comments.comment_author_name(comment)
+        if author == "an unknown user":
+            permission = None
+        elif author in self._ignore_permission_cache:
+            permission = self._ignore_permission_cache[author]
+        else:
+            path = (
+                f"repos/{self.config.owner}/{self.config.repository}/"
+                f"collaborators/{author}/permission"
+            )
+            try:
+                response = self.client.request(
+                    path=path,
+                    headers=self.config.headers,
+                    base_url=self.config.api_url,
+                )
+                result = response.json()
+                if not isinstance(result, dict) or not isinstance(
+                    result.get("permission"), str
+                ):
+                    log.warning("Unexpected GitHub repository permission response")
+                    permission = None
+                else:
+                    permission = (
+                        result["permission"].casefold() in self.WRITE_PERMISSIONS
+                    )
+            except Exception as error:
+                log.warning(
+                    f"Could not read GitHub repository permission for {author}: {error}"
+                )
+                permission = None
+            self._ignore_permission_cache[author] = permission
+
+        if permission is not None:
+            return permission
+        if self.ignore_authorization == "strict":
+            log.warning(
+                f"Rejecting @SocketSecurity ignore from {author}: GitHub repository "
+                "permission could not be read and --ignore-authorization is strict."
+            )
+            return False
+        log.warning(
+            f"Honoring @SocketSecurity ignore from {author} without verifying write "
+            "access: GitHub repository permission could not be read. Use a token "
+            "with repository metadata access, or --ignore-authorization strict to "
+            "reject instead."
+        )
+        return True
 
     def add_socket_comments(
         self,

socketsecurity/core/scm_comments.py

@@ -12,11 +12,6 @@
 class Comments:
     VIEW_REPORT_PATTERN = re.compile(r"\[View full report\]\(([^)\s]+)\)")
 
-    # GitHub stamps every issue comment with the author's relationship to the
-    # repository. Only these three imply write access; CONTRIBUTOR, MANNEQUIN,
-    # MENTIONEE, FIRST_TIMER, FIRST_TIME_CONTRIBUTOR and NONE do not.
-    WRITE_ACCESS_ASSOCIATIONS = frozenset({"OWNER", "MEMBER", "COLLABORATOR"})
-
     @staticmethod
     def comment_author_name(comment: Comment) -> str:
         """Best-effort display name for a comment author, across providers."""
@@ -151,10 +146,8 @@ def parse_alert_table_row(line: str) -> Optional[tuple[str, str, str]]:
 
         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.
+        outside this process's control. Malformed cells must not interrupt status
+        reporting. A row that cannot be read is a row whose alert stays reported.
         """
         cells = line.strip().lstrip("|").rstrip("|").split("|")
         if len(cells) != 5:

socketsecurity/output.py

@@ -229,6 +229,18 @@ 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."""
+        if (
+            getattr(diff_report, "is_full_scan", False)
+            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 = (

tests/core/test_full_scan_outputs.py

@@ -9,7 +9,9 @@
 
 from socketsecurity.config import CliConfig
 from socketsecurity.core import Core
+from socketsecurity.core.classes import Package
 from socketsecurity.core.socket_config import SocketConfig
+from socketsecurity.output import OutputHandler
 
 
 def _core(sdk, **cli_overrides):
@@ -61,6 +63,41 @@ def test_license_details_are_requested_for_the_scanned_packages(sdk, params):
     assert any("@" in component["purl"] for component in components)
 
 
+def test_license_details_preserve_package_namespace(sdk):
+    core = _core(sdk, generate_license=True)
+    package = Package(
+        id="artifact-id",
+        type="maven",
+        namespace="org.apache.logging.log4j",
+        name="log4j-core",
+        version="2.24.3",
+        score={},
+        alerts=[],
+    )
+    sdk.purl.post.return_value = [
+        {
+            "type": "maven",
+            "namespace": "org.apache.logging.log4j",
+            "name": "log4j-core",
+            "version": "2.24.3",
+            "licenseAttrib": [{"name": "Apache-2.0"}],
+            "licenseDetails": [{"license": "Apache-2.0"}],
+        }
+    ]
+
+    packages = core._add_license_details({package.id: package})
+
+    assert sdk.purl.post.call_args.kwargs["components"] == [
+        {
+            "purl": (
+                "pkg:/maven/org.apache.logging.log4j/"
+                "log4j-core@2.24.3"
+            )
+        }
+    ]
+    assert packages[package.id].licenseDetails == [{"license": "Apache-2.0"}]
+
+
 def test_alert_formats_still_fetch_the_sbom(sdk, params):
     core = _core(sdk, enable_json=True)
 
@@ -84,3 +121,8 @@ def test_console_only_run_skips_the_sbom_fetch(sdk, params):
     assert diff.packages == {}
     assert diff.new_alerts == []
     sdk.fullscans.stream.assert_not_called()
+
+    summary = OutputHandler(core.cli_config, sdk).build_summary_text(diff)
+    assert "No issues found" not in summary
+    assert "Findings were not fetched for console output" in summary
+    assert diff.report_url in summary