Skip to content

Commit 635bc7d

Browse files
leliaclaude
andcommitted
refactor: share one git remote parser between Buildkite consumers
The GitHub comment adapter and pull request link construction each parsed BUILDKITE_REPO independently. Consolidate on socketsecurity.core.git_remote, which also reports the remote host (needed for self-hosted GitHub Enterprise and GitLab) and preserves nested GitLab subgroup paths. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 0c1d564 commit 635bc7d

5 files changed

Lines changed: 109 additions & 62 deletions

File tree

docs/ci-cd.md

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -135,11 +135,12 @@ checked-out head commit. The CLI uses those local refs first and performs a
135135
targeted fetch only when a required ref or its comparison history is missing;
136136
it does not fetch every remote ref and tag during startup.
137137

138-
When `--scm github` is used from Buildkite, the CLI also derives GitHub comment
139-
context from `BUILDKITE_REPO`, `BUILDKITE_BUILD_CHECKOUT_PATH`, and the variables
140-
above. Set `GH_API_TOKEN` to a GitHub token with the required repository access.
141-
GitHub Enterprise users should also set `GITHUB_API_URL`; GitHub.com defaults to
142-
`https://api.github.com`.
138+
When `--scm github` is used from Buildkite, the CLI also posts GitHub PR comments.
139+
It identifies the repository from `BUILDKITE_REPO` and takes the rest of the build
140+
context from `BUILDKITE_BUILD_CHECKOUT_PATH` and the variables above — see
141+
[Buildkite PR context](#buildkite-pr-context). Set `GH_API_TOKEN` to a GitHub token
142+
with the required repository access. GitHub Enterprise users should also set
143+
`GITHUB_API_URL`; GitHub.com defaults to `https://api.github.com`.
143144

144145
#### Merge-base baselines in Buildkite (dynamic pipelines)
145146

@@ -246,13 +247,14 @@ the Buildkite platform example above. Buildkite sets `BUILDKITE_PULL_REQUEST` to
246247
`false` outside PR builds; the CLI treats that value as no PR.
247248

248249
Use `--integration github` for GitHub-hosted repositories and `--integration gitlab`
249-
for GitLab-hosted ones. In both cases the CLI reads the repository slug and host from
250-
[`BUILDKITE_REPO`](https://buildkite.com/docs/pipelines/configure/environment-variables#BUILDKITE_REPO)
251-
to build the pull request or merge request link, so github.com, GitLab.com, and
252-
self-hosted installations all work without extra configuration. Setting
253-
`CI_PROJECT_URL` still overrides the derived GitLab project URL. Keep `--scm api`
254-
unless you also intend to configure an existing GitHub or GitLab comment adapter and
255-
its provider token.
250+
for GitLab-hosted ones. The CLI identifies the repository from
251+
[`BUILDKITE_REPO`](https://buildkite.com/docs/pipelines/configure/environment-variables#BUILDKITE_REPO),
252+
taking both the slug and the host from it, so github.com, GitLab.com, and self-hosted
253+
installations all build a correct pull request or merge request link without extra
254+
configuration. That same value identifies the repository for GitHub PR comments when
255+
`--scm github` is set. `CI_PROJECT_URL` still overrides the derived GitLab project URL.
256+
Keep `--scm api` unless you also intend to configure an existing GitHub or GitLab
257+
comment adapter and its provider token.
256258

257259
`--scm github` and `--scm gitlab` also imply the matching scan integration for
258260
Dashboard metadata unless `--integration` was explicitly supplied. PR comments

socketsecurity/core/git_remote.py

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
"""Parsing for git remote URLs.
2+
3+
CI systems that are not tied to a single SCM expose the checkout URL rather than
4+
an ``owner/repo`` slug (Buildkite's ``BUILDKITE_REPO``, for example). Both the
5+
GitHub comment adapter and pull request context resolution need to recover the
6+
slug from it, so the parsing lives here rather than in either caller.
7+
"""
8+
import re
9+
from typing import Optional, Tuple
10+
from urllib.parse import urlparse
11+
12+
# git@host:owner/repo - the scp-like syntax urlparse cannot handle. The negative
13+
# lookahead keeps scheme-prefixed URLs (https://, ssh://) out of this branch.
14+
_SCP_LIKE_REMOTE = re.compile(r"^(?:[^@/]+@)?([^:/]+):(?!//)(.+)$")
15+
16+
17+
def parse_git_remote(value: Optional[str]) -> Tuple[Optional[str], Optional[str]]:
18+
"""Split a git remote URL into its host and its repository path.
19+
20+
Returns ``(host, path)``, or ``(None, None)`` when the value is not a usable
21+
remote. The path is returned whole rather than as ``owner``/``repo`` because
22+
GitLab projects can be nested under subgroups; callers that only want the
23+
last two segments can split it themselves. ``host`` is ``None`` for a bare
24+
``owner/repo`` path, which carries no host to report.
25+
"""
26+
if not value:
27+
return None, None
28+
url = value.strip().rstrip("/")
29+
if url.endswith(".git"):
30+
url = url[:-4]
31+
32+
match = _SCP_LIKE_REMOTE.match(url)
33+
if match:
34+
return match.group(1), match.group(2).strip("/")
35+
36+
parsed = urlparse(url)
37+
if parsed.scheme in ("http", "https", "ssh", "git") and parsed.hostname:
38+
return parsed.hostname, parsed.path.strip("/")
39+
40+
# A bare owner/repo path, with no scheme and nothing to infer a host from.
41+
if "/" in url:
42+
return None, url.strip("/")
43+
return None, None

socketsecurity/core/pull_request.py

Lines changed: 4 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
from typing import Mapping, Optional
44
from urllib.parse import urlparse
55

6+
from socketsecurity.core.git_remote import parse_git_remote
7+
68

79
@dataclass(frozen=True)
810
class PullRequestContext:
@@ -28,35 +30,6 @@ def _repository_url(value: Optional[str]) -> Optional[str]:
2830
return url if parsed.scheme in ("http", "https") and parsed.netloc else None
2931

3032

31-
# git@host:owner/repo - the scp-like syntax urlparse cannot handle. The negative
32-
# lookahead keeps scheme-prefixed URLs (https://, ssh://) out of this branch.
33-
_SCP_LIKE_REMOTE = re.compile(r"^(?:[^@/]+@)?([^:/]+):(?!//)(.+)$")
34-
35-
36-
def _parse_remote(value: Optional[str]) -> tuple[Optional[str], Optional[str]]:
37-
"""Split a git remote URL into its host and its ``owner/repo`` path.
38-
39-
Providers expose the checkout URL rather than a slug on CI systems that are
40-
not tied to a single SCM (Buildkite's ``BUILDKITE_REPO``, for example), so
41-
the slug the URL builders need has to be recovered from it. The path is
42-
returned whole because GitLab projects can be nested under subgroups.
43-
"""
44-
if not value:
45-
return None, None
46-
url = value.strip().rstrip("/")
47-
if url.endswith(".git"):
48-
url = url[:-4]
49-
50-
match = _SCP_LIKE_REMOTE.match(url)
51-
if match:
52-
return match.group(1), match.group(2).strip("/")
53-
54-
parsed = urlparse(url)
55-
if parsed.scheme in ("http", "https", "ssh", "git") and parsed.hostname:
56-
return parsed.hostname, parsed.path.strip("/")
57-
return None, None
58-
59-
6033
def _github_number(env: Mapping[str, str]) -> int:
6134
number = _positive_int(env.get("PR_NUMBER"))
6235
if number:
@@ -66,7 +39,7 @@ def _github_number(env: Mapping[str, str]) -> int:
6639

6740

6841
def _github_url(number: int, repo: Optional[str], env: Mapping[str, str]) -> Optional[str]:
69-
remote_host, remote_path = _parse_remote(env.get("BUILDKITE_REPO"))
42+
remote_host, remote_path = parse_git_remote(env.get("BUILDKITE_REPO"))
7043
# config.repo is only ever a bare repository name, so it cannot produce a
7144
# slug on its own; it is kept last for callers that pass a full owner/repo.
7245
repository = env.get("GITHUB_REPOSITORY") or remote_path or repo
@@ -80,7 +53,7 @@ def _github_url(number: int, repo: Optional[str], env: Mapping[str, str]) -> Opt
8053
def _gitlab_url(number: int, repo: Optional[str], env: Mapping[str, str]) -> Optional[str]:
8154
project_url = _repository_url(env.get("CI_PROJECT_URL"))
8255
if not project_url:
83-
remote_host, remote_path = _parse_remote(env.get("BUILDKITE_REPO"))
56+
remote_host, remote_path = parse_git_remote(env.get("BUILDKITE_REPO"))
8457
project_path = env.get("CI_PROJECT_PATH") or remote_path or repo
8558
server = env.get("CI_SERVER_URL") or (f"https://{remote_host}" if remote_host else "")
8659
server = server.rstrip("/")

socketsecurity/core/scm/github.py

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
import json
22
import os
33
import sys
4-
import urllib.parse
54
from dataclasses import dataclass
65

76
from git import Optional
87

98
from socketsecurity import USER_AGENT
109
from socketsecurity.core import log
1110
from socketsecurity.core.classes import Comment
11+
from socketsecurity.core.git_remote import parse_git_remote
1212
from socketsecurity.core.scm_comments import Comments
1313
from socketsecurity.socketcli import CliClient
1414

@@ -38,24 +38,12 @@ class GithubConfig:
3838
@staticmethod
3939
def _repository_from_buildkite() -> tuple[str, str]:
4040
"""Return ``(owner, repository)`` from Buildkite's Git repository URL."""
41-
repository_url = (
42-
# Comments and statuses belong to the pipeline/base repository,
43-
# not a contributor's fork from BUILDKITE_PULL_REQUEST_REPO.
44-
os.getenv("BUILDKITE_REPO")
45-
or os.getenv("BUILDKITE_PULL_REQUEST_REPO")
46-
or ""
47-
).strip()
48-
if not repository_url:
49-
return "", ""
50-
51-
if "://" in repository_url:
52-
repository_path = urllib.parse.urlparse(repository_url).path
53-
elif ":" in repository_url:
54-
# SCP-style SSH URL: git@github.com:owner/repository.git
55-
repository_path = repository_url.split(":", 1)[1]
56-
else:
57-
repository_path = repository_url
58-
parts = repository_path.strip("/").removesuffix(".git").split("/")
41+
# Comments and statuses belong to the pipeline/base repository, not a
42+
# contributor's fork from BUILDKITE_PULL_REQUEST_REPO.
43+
_, repository_path = parse_git_remote(
44+
os.getenv("BUILDKITE_REPO") or os.getenv("BUILDKITE_PULL_REQUEST_REPO")
45+
)
46+
parts = repository_path.split("/") if repository_path else []
5947
if len(parts) < 2:
6048
return "", ""
6149
return parts[-2], parts[-1]

tests/unit/test_git_remote.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
"""Tests for the shared git remote parser.
2+
3+
Both the GitHub comment adapter (`GithubConfig._repository_from_buildkite`) and
4+
pull request URL construction depend on this, so the URL forms Buildkite and
5+
self-hosted installations emit are pinned here rather than in either caller.
6+
"""
7+
import pytest
8+
9+
from socketsecurity.core.git_remote import parse_git_remote
10+
11+
12+
@pytest.mark.parametrize(
13+
("remote", "expected"),
14+
[
15+
# The three forms BUILDKITE_REPO is observed to take.
16+
("git@github.com:acme/widgets.git", ("github.com", "acme/widgets")),
17+
("https://github.com/acme/widgets.git", ("github.com", "acme/widgets")),
18+
("ssh://git@github.com/acme/widgets.git", ("github.com", "acme/widgets")),
19+
# Self-hosted hosts must survive: they decide the PR/MR link's origin.
20+
("git@github.example.com:acme/widgets.git", ("github.example.com", "acme/widgets")),
21+
("https://gitlab.example.com/acme/widgets", ("gitlab.example.com", "acme/widgets")),
22+
# GitLab subgroups: the path is returned whole, not just the last two parts.
23+
(
24+
"ssh://git@gitlab.example.com/acme/platform/widgets.git",
25+
("gitlab.example.com", "acme/platform/widgets"),
26+
),
27+
("git://github.com/acme/widgets.git", ("github.com", "acme/widgets")),
28+
# Cosmetic variation callers should not have to normalise themselves.
29+
(" https://github.com/acme/widgets/ ", ("github.com", "acme/widgets")),
30+
# Credentials in the URL must not leak into the host.
31+
("https://user@github.com/acme/widgets", ("github.com", "acme/widgets")),
32+
# A bare slug carries no host to report, but is still usable.
33+
("acme/widgets", (None, "acme/widgets")),
34+
# Nothing usable.
35+
("not-a-repository", (None, None)),
36+
("", (None, None)),
37+
(None, (None, None)),
38+
],
39+
)
40+
def test_parse_git_remote(remote, expected):
41+
assert parse_git_remote(remote) == expected

0 commit comments

Comments
 (0)