Skip to content

Commit 800fde7

Browse files
committed
Build PR comment manifest links from the full repository URL
Manifest links in GitHub, GitLab and Bitbucket comments were built as {server}/{config.repo}/blob/..., but config.repo is the Socket repository slug, which has no owner, so every link pointed at a path that doesn't exist. The ignore telemetry's owner field split config.repo on "/" and was always empty for the same reason. Resolve the repository's web URL once, from the CI environment and then the checkout's origin remote, reusing the lookups the pull request URL already does. Manifest links and the telemetry owner use it, and links fall back to the Socket report when it can't be resolved.
1 parent 8f38c02 commit 800fde7

9 files changed

Lines changed: 165 additions & 36 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,12 @@
88
`my-org/my-repo`). A value containing `/` now exits `1` and names the slug to
99
use. Pass the owner with `--workspace`.
1010

11+
### Fixed: manifest links in pull request comments
12+
13+
- Manifest links in GitHub, GitLab and Bitbucket comments now include the
14+
repository owner. When the CLI can't determine the owner, they link to the
15+
Socket report instead.
16+
1117
### Deprecated: `--pending-head`
1218

1319
- `--pending-head` has no effect and logs a deprecation warning. Use

‎socketsecurity/config.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,8 @@ class CliConfig:
197197
sub_paths: List[str] = field(default_factory=list)
198198
workspace_name: Optional[str] = None
199199
workspace: Optional[str] = None
200+
# Web URL of the scanned repository, resolved at runtime; see resolve_repository_url.
201+
repository_url: Optional[str] = None
200202
# Reachability Flags
201203
reach: bool = False
202204
reach_version: Optional[str] = None

‎socketsecurity/core/git_interface.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,10 @@ def __init__(self, path: str, base_commit_sha: str | None = None):
7474
log.debug(f"Commit committer: {self.commit.committer.name} <{self.commit.committer.email}>")
7575

7676
# Extract repository name from git remote, with fallback to default
77+
self.remote_url: str | None = None
7778
try:
7879
remote_url = self.repo.remotes.origin.url
80+
self.remote_url = remote_url
7981
self.repo_name = remote_url.split('.git')[0].split('/')[-1]
8082
log.debug(f"Repository name detected from git remote: {self.repo_name}")
8183
except Exception as error:

‎socketsecurity/core/messages.py‎

Lines changed: 6 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import json
22
import logging
3-
import os
43
import re
54
import uuid
65
from datetime import datetime, timezone
@@ -94,30 +93,12 @@ def get_manifest_file_url(diff: Diff, manifest_path: str, config=None) -> str:
9493

9594
# Generate URL based on SCM type using config information
9695
# NEVER use diff.diff_url for SCM URLs - those are Socket URLs for "View report" links
97-
if scm_type == "github":
98-
if config and hasattr(config, 'repo') and config.repo:
99-
# Get branch from config, default to main
100-
branch = getattr(config, 'branch', 'main') if hasattr(config, 'branch') and config.branch else 'main'
101-
# Construct GitHub URL from repo info (could be github.com or GitHub Enterprise)
102-
github_server = os.getenv('GITHUB_SERVER_URL', 'https://github.com')
103-
return f"{github_server}/{config.repo}/blob/{branch}/{clean_path}"
104-
105-
elif scm_type == "gitlab":
106-
if config and hasattr(config, 'repo') and config.repo:
107-
# Get branch from config, default to main
108-
branch = getattr(config, 'branch', 'main') if hasattr(config, 'branch') and config.branch else 'main'
109-
# Construct GitLab URL from repo info (could be gitlab.com or self-hosted GitLab)
110-
gitlab_server = os.getenv('CI_SERVER_URL', 'https://gitlab.com')
111-
return f"{gitlab_server}/{config.repo}/-/blob/{branch}/{clean_path}"
112-
113-
elif scm_type == "bitbucket":
114-
if config and hasattr(config, 'repo') and config.repo:
115-
# Get branch from config, default to main
116-
branch = getattr(config, 'branch', 'main') if hasattr(config, 'branch') and config.branch else 'main'
117-
# Construct Bitbucket URL from repo info (could be bitbucket.org or Bitbucket Server)
118-
bitbucket_server = os.getenv('BITBUCKET_SERVER_URL', 'https://bitbucket.org')
119-
return f"{bitbucket_server}/{config.repo}/src/{branch}/{clean_path}"
120-
96+
repository_url = getattr(config, 'repository_url', None) if config else None
97+
if repository_url and scm_type in ("github", "gitlab", "bitbucket"):
98+
branch = getattr(config, 'branch', None) or 'main'
99+
blob_path = {"github": "blob", "gitlab": "-/blob", "bitbucket": "src"}[scm_type]
100+
return f"{repository_url}/{blob_path}/{branch}/{clean_path}"
101+
121102
# Fallback to Socket file view for API or unknown repository types
122103
if hasattr(diff, 'report_url') and diff.report_url:
123104
# Strip leading slash and URL encode for Socket dashboard

‎socketsecurity/core/pull_request.py‎

Lines changed: 69 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,10 @@ def _github_number(env: Mapping[str, str]) -> int:
5959
return parse_pull_request_number(match.group(1)) if match else 0
6060

6161

62-
def _github_url(number: int, repo: Optional[str], env: Mapping[str, str]) -> Optional[str]:
63-
remote_host, remote_path = parse_git_remote(env.get("BUILDKITE_REPO"))
62+
def _github_repository_url(
63+
repo: Optional[str], env: Mapping[str, str], remote_url: Optional[str] = None
64+
) -> Optional[str]:
65+
remote_host, remote_path = parse_git_remote(env.get("BUILDKITE_REPO") or remote_url)
6466
# config.repo is only ever a bare repository name, so it cannot produce a
6567
# slug on its own; it is kept last for callers that pass a full owner/repo.
6668
repository = env.get("GITHUB_REPOSITORY") or remote_path or repo
@@ -71,20 +73,82 @@ def _github_url(number: int, repo: Optional[str], env: Mapping[str, str]) -> Opt
7173
or (_http_url(f"https://{remote_host}") if remote_host else None)
7274
or "https://github.com"
7375
)
74-
return f"{server}/{repository.strip('/')}/pull/{number}"
76+
return f"{server}/{repository.strip('/')}"
7577

7678

77-
def _gitlab_url(number: int, repo: Optional[str], env: Mapping[str, str]) -> Optional[str]:
79+
def _gitlab_project_url(
80+
repo: Optional[str], env: Mapping[str, str], remote_url: Optional[str] = None
81+
) -> Optional[str]:
7882
project_url = _repository_url(env.get("CI_PROJECT_URL"))
7983
if not project_url:
80-
remote_host, remote_path = parse_git_remote(env.get("BUILDKITE_REPO"))
84+
remote_host, remote_path = parse_git_remote(env.get("BUILDKITE_REPO") or remote_url)
8185
project_path = env.get("CI_PROJECT_PATH") or remote_path or repo
8286
server = (
8387
_http_url(env.get("CI_SERVER_URL"))
8488
or (_http_url(f"https://{remote_host}") if remote_host else None)
8589
)
8690
if server and project_path and "/" in project_path:
8791
project_url = f"{server}/{project_path.strip('/')}"
92+
return project_url
93+
94+
95+
def _bitbucket_repository_url(
96+
repo: Optional[str], env: Mapping[str, str], remote_url: Optional[str] = None
97+
) -> Optional[str]:
98+
remote_host, remote_path = parse_git_remote(remote_url)
99+
repository = env.get("BITBUCKET_REPO_FULL_NAME") or remote_path or repo
100+
if not repository or "/" not in repository:
101+
return None
102+
server = (
103+
_http_url(env.get("BITBUCKET_SERVER_URL"))
104+
or (_http_url(f"https://{remote_host}") if remote_host else None)
105+
or "https://bitbucket.org"
106+
)
107+
return f"{server}/{repository.strip('/')}"
108+
109+
110+
def resolve_repository_url(
111+
scm: Optional[str],
112+
repo: Optional[str] = None,
113+
env: Optional[Mapping[str, str]] = None,
114+
remote_url: Optional[str] = None,
115+
) -> Optional[str]:
116+
"""Return the web URL of the scanned repository, such as ``https://github.com/owner/repo``.
117+
118+
The CI environment wins, then the checkout's ``origin`` remote. ``None`` means the
119+
owner could not be determined: ``config.repo`` is the Socket repository slug,
120+
which has no owner, so a link built from it alone would point nowhere.
121+
"""
122+
environment = env or {}
123+
provider = str(scm or "").lower()
124+
if provider == "github":
125+
return _github_repository_url(repo, environment, remote_url)
126+
if provider == "gitlab":
127+
return _gitlab_project_url(repo, environment, remote_url)
128+
if provider == "bitbucket":
129+
return _bitbucket_repository_url(repo, environment, remote_url)
130+
return None
131+
132+
133+
def repository_owner(repository_url: Optional[str]) -> str:
134+
"""Return everything before the repository name in ``repository_url``'s path.
135+
136+
That is the owner on GitHub and Bitbucket, and the full group path on GitLab,
137+
where projects can be nested under subgroups.
138+
"""
139+
if not repository_url:
140+
return ""
141+
path = urlparse(repository_url).path.strip("/")
142+
return path.rsplit("/", 1)[0] if "/" in path else ""
143+
144+
145+
def _github_url(number: int, repo: Optional[str], env: Mapping[str, str]) -> Optional[str]:
146+
repository_url = _github_repository_url(repo, env)
147+
return f"{repository_url}/pull/{number}" if repository_url else None
148+
149+
150+
def _gitlab_url(number: int, repo: Optional[str], env: Mapping[str, str]) -> Optional[str]:
151+
project_url = _gitlab_project_url(repo, env)
88152
return f"{project_url}/-/merge_requests/{number}" if project_url else None
89153

90154

‎socketsecurity/socketcli.py‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@
2222
from socketsecurity.core.messages import Messages
2323
from socketsecurity.core.pull_request import (
2424
parse_pull_request_number,
25+
repository_owner,
2526
resolve_pull_request_context,
27+
resolve_repository_url,
2628
)
2729
from socketsecurity.core.scm_comments import Comments
2830
from socketsecurity.core.socket_config import SocketConfig, module_folder_dirs
@@ -601,6 +603,12 @@ def main_code():
601603
from socketsecurity.core.scm.gitlab import Gitlab, GitlabConfig
602604
gitlab_config = GitlabConfig.from_env()
603605
scm = Gitlab(client=client, config=gitlab_config, ignore_authorization=config.ignore_authorization)
606+
config.repository_url = resolve_repository_url(
607+
config.scm,
608+
config.repo,
609+
os.environ,
610+
remote_url=getattr(git_repo, "remote_url", None) if is_repo else None,
611+
)
604612
# Don't override config.default_branch if it was explicitly set via --default-branch flag
605613
# Only use SCM detection if --default-branch wasn't provided
606614
if scm is not None and not config.default_branch:
@@ -806,7 +814,7 @@ def _is_unprocessed(c):
806814
"alert_action": "error",
807815
"event_sender_created_at": now,
808816
"vcs_provider": integration_type,
809-
"owner": config.repo.split("/")[0] if "/" in config.repo else "",
817+
"owner": repository_owner(config.repository_url),
810818
"repo": config.repo,
811819
"pr_number": pr_number,
812820
"ignore_all": ignore_all,
@@ -881,7 +889,7 @@ def _is_unprocessed(c):
881889
"client_action": "ignore",
882890
"event_sender_created_at": now,
883891
"vcs_provider": integration_type,
884-
"owner": config.repo.split("/")[0] if "/" in config.repo else "",
892+
"owner": repository_owner(config.repository_url),
885893
"repo": config.repo,
886894
"pr_number": pr_number,
887895
"ignore_all": c_ignore_all,

‎tests/unit/test_messages.py‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,3 +79,26 @@ def test_security_comment_omits_missing_patched_version():
7979
comment = Messages.security_comment_template(diff)
8080

8181
assert "Patched version:" not in comment
82+
83+
84+
class _LinkConfig:
85+
scm = "github"
86+
branch = "main"
87+
repository_url = "https://github.com/acme/widgets"
88+
89+
90+
def test_manifest_link_uses_the_full_repository_url():
91+
url = Messages.get_manifest_file_url(Diff(), "app/package.json", _LinkConfig())
92+
93+
assert url == "https://github.com/acme/widgets/blob/main/app/package.json"
94+
95+
96+
def test_manifest_link_falls_back_to_the_report_without_a_repository_url():
97+
"""A bare slug has no owner, so the link goes to the Socket report instead."""
98+
config = _LinkConfig()
99+
config.repository_url = None
100+
diff = Diff(report_url="https://socket.dev/report/123")
101+
102+
url = Messages.get_manifest_file_url(diff, "app/package.json", config)
103+
104+
assert url == "https://socket.dev/report/123?tab=files&file=app%2Fpackage.json"

‎tests/unit/test_pr_comment_rendering.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -376,11 +376,11 @@ def test_returns_empty_when_absent(self):
376376
class _RepoConfig(_FakeConfig):
377377
"""A config that reaches the branch which embeds the path verbatim.
378378
379-
Without repo/branch, get_manifest_file_url returns "" or a percent-encoded
380-
Socket link, and the path never lands in the comment -- so a test using the
381-
bare config asserts nothing.
379+
Without repository_url/branch, get_manifest_file_url returns "" or a
380+
percent-encoded Socket link, and the path never lands in the comment -- so a
381+
test using the bare config asserts nothing.
382382
"""
383-
repo: str = "acme/widgets"
383+
repository_url: str = "https://github.com/acme/widgets"
384384
branch: str = "main"
385385

386386

‎tests/unit/test_pull_request_context.py‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22

33
from socketsecurity.core.pull_request import (
44
parse_pull_request_number,
5+
repository_owner,
56
resolve_pull_request_context,
7+
resolve_repository_url,
68
)
79

810

@@ -291,3 +293,44 @@ def test_bare_repository_name_alone_yields_no_url():
291293

292294
assert context.number == 42
293295
assert context.url is None
296+
297+
298+
@pytest.mark.parametrize(
299+
("scm", "env", "remote_url", "expected"),
300+
[
301+
("github", {"GITHUB_REPOSITORY": "acme/widgets"}, None, "https://github.com/acme/widgets"),
302+
(
303+
"github",
304+
{"GITHUB_REPOSITORY": "acme/widgets", "GITHUB_SERVER_URL": "https://ghe.example.com"},
305+
None,
306+
"https://ghe.example.com/acme/widgets",
307+
),
308+
(
309+
"gitlab",
310+
{"CI_PROJECT_URL": "https://gitlab.example.com/group/sub/widgets"},
311+
None,
312+
"https://gitlab.example.com/group/sub/widgets",
313+
),
314+
("bitbucket", {"BITBUCKET_REPO_FULL_NAME": "acme/widgets"}, None, "https://bitbucket.org/acme/widgets"),
315+
# Outside CI, the origin remote supplies the host and path; credentials are dropped.
316+
("github", {}, "https://x-token:secret@github.com/acme/widgets.git", "https://github.com/acme/widgets"),
317+
("gitlab", {}, "git@gitlab.com:group/widgets.git", "https://gitlab.com/group/widgets"),
318+
# A bare slug has no owner, so there is no repository URL to build.
319+
("github", {}, None, None),
320+
("api", {"GITHUB_REPOSITORY": "acme/widgets"}, None, None),
321+
],
322+
)
323+
def test_resolve_repository_url(scm, env, remote_url, expected):
324+
assert resolve_repository_url(scm, "widgets", env, remote_url=remote_url) == expected
325+
326+
327+
@pytest.mark.parametrize(
328+
("url", "owner"),
329+
[
330+
("https://github.com/acme/widgets", "acme"),
331+
("https://gitlab.com/group/sub/widgets", "group/sub"),
332+
(None, ""),
333+
],
334+
)
335+
def test_repository_owner(url, owner):
336+
assert repository_owner(url) == owner

0 commit comments

Comments
 (0)