Determine SCM type from config instead of sniffing the Socket report URL

CodeQL flagged `"github.com" in diff_url` as incomplete URL substring sanitization. Looking at what diff_url actually holds makes the finding more interesting than a sanitization gap. diff_url is always a Socket dashboard link, built in Core as `https://socket.dev/dashboard/org/{org_slug}/diff/...` (or the equivalent sbom URL). Its host is always socket.dev and it carries no SCM information -- which is exactly what the comment three lines below the check already said. The only variable part is the org slug, so the sniff could only ever fire when a Socket org slug happened to contain "github", "gitlab" or "bitbucket". Such an org got a link to a repository host it may not use; everyone else fell through to the Socket file view. The branch was also almost unreachable: CliConfig declares `scm` with a default of "api", so `hasattr(config, "scm")` is true for every real config and the elif never runs. It was observable only for a config object carrying `repo` but no `scm`, since the URL builders all require a truthy config -- with `config=None` the sniffed value was computed and then discarded. Replaced with `getattr(config, "scm", None) or "api"`, which handles a missing config, a config without the attribute, and an empty value. Adds tests for get_manifest_file_url, which had none: GitHub, GitHub Enterprise, GitLab, self-hosted GitLab, Bitbucket, the Socket fallback, build-agent prefix stripping, and multi-manifest paths. The three org-slug cases are regression guards, confirmed to fail against the old implementation. Removing the dead branch drops the function under the complexity limit, so RUF100 required its `# noqa: C901` be removed. The backlog is now 19. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
SocketDev · lelia · Sep 4, 2026 · Sep 4, 2026 · Sep 7, 2026 · Sep 7, 2026
commit 3742c396dfb8d052a6e6e7b7a0b8c44bdae39dd3
@@ -34,7 +34,7 @@ def map_severity_to_sarif(severity: str) -> str:
         return severity_mapping.get(severity.lower(), "note")
 
     @staticmethod
-    def get_manifest_file_url(diff: Diff, manifest_path: str, config=None) -> str:  # noqa: C901
+    def get_manifest_file_url(diff: Diff, manifest_path: str, config=None) -> str:
         """
         Generate proper URL for manifest file based on the repository type and diff URL.
 
@@ -71,18 +71,16 @@ def get_manifest_file_url(diff: Diff, manifest_path: str, config=None) -> str:
         # Remove leading slashes
         clean_path = clean_path.lstrip("/")
 
-        # Determine SCM type from config or diff_url
-        scm_type = "api"  # Default to API
-        if config and hasattr(config, "scm"):
-            scm_type = config.scm.lower()
-        elif hasattr(diff, "diff_url") and diff.diff_url:
-            diff_url = diff.diff_url.lower()
-            if "github.com" in diff_url or "github" in diff_url:
-                scm_type = "github"
-            elif "gitlab" in diff_url:
-                scm_type = "gitlab"
-            elif "bitbucket" in diff_url:
-                scm_type = "bitbucket"
+        # Determine SCM type from config.
+        #
+        # diff.diff_url is deliberately not consulted. It is always a Socket
+        # dashboard link -- https://socket.dev/dashboard/org/<org>/diff/... , as
+        # the note below says -- so it carries no SCM information at all. The
+        # substring sniff that used to live here ("github" in diff_url) could
+        # therefore only fire when the Socket *org slug* happened to contain
+        # "github", "gitlab" or "bitbucket", mislabelling the SCM for those orgs
+        # and doing nothing for everyone else.
+        scm_type = (getattr(config, "scm", None) or "api").lower()
 
         # Generate URL based on SCM type using config information
         # NEVER use diff.diff_url for SCM URLs - those are Socket URLs for "View report" links

@@ -0,0 +1,127 @@
+"""Tests for Messages.get_manifest_file_url.
+
+The SCM type comes from config alone. It used to fall back to sniffing
+`diff.diff_url` for the substrings "github" / "gitlab" / "bitbucket", which
+CodeQL flagged as incomplete URL sanitization. The deeper problem was that
+diff_url is always a Socket dashboard link, so the only variable part it
+contains is the org slug -- meaning the sniff mislabelled the SCM for any org
+whose slug happened to contain one of those words, and did nothing otherwise.
+"""
+
+from dataclasses import dataclass
+
+import pytest
+
+from socketsecurity.core.classes import Diff
+from socketsecurity.core.messages import Messages
+
+SOCKET_REPORT = "https://socket.dev/dashboard/org/acme/sbom/abc123"
+
+
+@dataclass
+class _Config:
+    scm: str = "api"
+    repo: str = "acme/widgets"
+    branch: str = "main"
+
+
+def _diff(diff_url: str = "https://socket.dev/dashboard/org/acme/diff/h1/abc123") -> Diff:
+    return Diff(id="abc123", diff_url=diff_url, report_url=SOCKET_REPORT)
+
+
+def test_github_url_is_built_from_config_not_diff_url():
+    url = Messages.get_manifest_file_url(_diff(), "package.json", _Config(scm="github"))
+    assert url == "https://github.com/acme/widgets/blob/main/package.json"
+
+
+def test_github_enterprise_honours_server_env(monkeypatch):
+    monkeypatch.setenv("GITHUB_SERVER_URL", "https://github.mycorp.com")
+    url = Messages.get_manifest_file_url(_diff(), "package.json", _Config(scm="github"))
+    assert url == "https://github.mycorp.com/acme/widgets/blob/main/package.json"
+
+
+def test_gitlab_uses_blob_path_and_server_env(monkeypatch):
+    monkeypatch.setenv("CI_SERVER_URL", "https://gitlab.mycorp.com")
+    url = Messages.get_manifest_file_url(_diff(), "go.mod", _Config(scm="gitlab", branch="dev"))
+    assert url == "https://gitlab.mycorp.com/acme/widgets/-/blob/dev/go.mod"
+
+
+def test_bitbucket_uses_src_path():
+    url = Messages.get_manifest_file_url(_diff(), "pom.xml", _Config(scm="bitbucket"))
+    assert url == "https://bitbucket.org/acme/widgets/src/main/pom.xml"
+
+
+def test_scm_is_case_insensitive():
+    url = Messages.get_manifest_file_url(_diff(), "package.json", _Config(scm="GitHub"))
+    assert url.startswith("https://github.com/")
+
+
+@pytest.mark.parametrize("scm", ["api", "", "unknown"])
+def test_non_scm_types_fall_back_to_the_socket_file_view(scm):
+    url = Messages.get_manifest_file_url(_diff(), "src/package.json", _Config(scm=scm))
+    assert url == f"{SOCKET_REPORT}?tab=files&file=src%2Fpackage.json"
+
+
+def test_missing_config_falls_back_to_the_socket_file_view():
+    url = Messages.get_manifest_file_url(_diff(), "package.json", None)
+    assert url == f"{SOCKET_REPORT}?tab=files&file=package.json"
+
+
+class _ConfigWithoutScm:
+    """A config that carries repo/branch but no `scm` attribute.
+
+    This is the shape that made the old diff_url sniff observable: it is truthy
+    and has `repo`, so a sniffed scm_type actually reached the URL builders.
+    With `config=None` the sniffed value was computed and then discarded,
+    because every branch also required a truthy config.
+    """
+
+    repo = "acme/widgets"
+    branch = "main"
+
+
+def test_config_without_an_scm_attribute_falls_back_to_socket():
+    url = Messages.get_manifest_file_url(_diff(), "package.json", _ConfigWithoutScm())
+    assert url == f"{SOCKET_REPORT}?tab=files&file=package.json"
+
+
+@pytest.mark.parametrize(
+    "diff_url",
+    [
+        "https://socket.dev/dashboard/org/github-tools/diff/h1/abc123",
+        "https://socket.dev/dashboard/org/our-gitlab-org/diff/h1/abc123",
+        "https://socket.dev/dashboard/org/bitbucket-team/diff/h1/abc123",
+    ],
+)
+def test_org_slug_containing_an_scm_name_does_not_change_the_url(diff_url):
+    """Regression guard: diff_url must not influence SCM detection.
+
+    Each of these Socket org slugs embeds an SCM name. The old substring sniff
+    read that as the repository's SCM and emitted a GitHub/GitLab/Bitbucket
+    link for orgs that may use none of them. Uses a config without `scm` so the
+    sniffed value would actually be reached.
+    """
+    url = Messages.get_manifest_file_url(_diff(diff_url), "package.json", _ConfigWithoutScm())
+    assert url == f"{SOCKET_REPORT}?tab=files&file=package.json"
+
+
+def test_first_manifest_is_used_when_several_are_joined():
+    url = Messages.get_manifest_file_url(_diff(), "a/package.json;b/yarn.lock", _Config(scm="github"))
+    assert url == "https://github.com/acme/widgets/blob/main/a/package.json"
+
+
+@pytest.mark.parametrize(
+    "raw",
+    [
+        "home/runner/work/widgets/widgets/src/package.json",
+        "/home/runner/work/widgets/widgets/src/package.json",
+        "opt/buildagent/work/abc123/widgets/src/package.json",
+    ],
+)
+def test_build_agent_prefixes_are_stripped(raw):
+    url = Messages.get_manifest_file_url(_diff(), raw, _Config(scm="github"))
+    assert url == "https://github.com/acme/widgets/blob/main/src/package.json"
+
+
+def test_empty_manifest_path_returns_empty_string():
+    assert Messages.get_manifest_file_url(_diff(), "", _Config(scm="github")) == ""