Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/install/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func detectPlatform(cloneURL string) Platform {
if host == "" {
return PlatformUnknown
}
if strings.Contains(host, "github") {
if isGitHubLikeHost(host) {
return PlatformGitHub
}
if strings.Contains(host, "gitlab") {
Expand Down
2 changes: 2 additions & 0 deletions internal/install/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ func TestDetectPlatform(t *testing.T) {
}{
{"github.com", "https://github.com/org/repo.git", PlatformGitHub},
{"github enterprise", "https://github.mycompany.com/org/repo.git", PlatformGitHub},
{"github enterprise data residency", "https://acme.ghe.com/org/repo.git", PlatformGitHub},
{"github enterprise data residency ssh", "acme@acme.ghe.com:org/repo.git", PlatformGitHub},
{"gitlab.com", "https://gitlab.com/org/repo.git", PlatformGitLab},
{"self-hosted gitlab", "https://gitlab.internal.co/org/repo.git", PlatformGitLab},
{"bitbucket.org", "https://bitbucket.org/team/repo.git", PlatformBitbucket},
Expand Down
1 change: 1 addition & 0 deletions internal/install/conflict_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ func TestNormalizeCloneURL(t *testing.T) {
{"https://github.com/owner/repo", "github.com/owner/repo"},
{"git@github.com:owner/repo.git", "github.com/owner/repo"},
{"git@github.com:owner/repo", "github.com/owner/repo"},
{"acme@acme.ghe.com:MyOrg/repo.git", "acme.ghe.com/myorg/repo"},
{"https://github.com/Owner/Repo.git", "github.com/owner/repo"},
{"https://github.com/owner/repo/", "github.com/owner/repo"},
{"http://github.com/owner/repo.git", "github.com/owner/repo"},
Expand Down
7 changes: 2 additions & 5 deletions internal/install/github_download.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,13 +246,10 @@ func gitHubAPIBase(source *Source) (string, error) {
if host == "" {
return "", fmt.Errorf("unable to determine source host")
}
if !strings.Contains(host, "github") {
if !isGitHubLikeHost(host) {
return "", fmt.Errorf("source host %q is not GitHub-compatible", host)
}
if host == "github.com" {
return "https://api.github.com", nil
}
return fmt.Sprintf("https://%s/api/v3", host), nil
return gitHubAPIBaseForHost(host), nil
}

func buildGitHubContentsURL(apiBase, owner, repo, path string) string {
Expand Down
5 changes: 5 additions & 0 deletions internal/install/github_download_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,11 @@ func TestGitHubAPIBase(t *testing.T) {
source: &Source{CloneURL: "https://github.acme.com/acme/repo.git"},
want: "https://github.acme.com/api/v3",
},
{
name: "ghe data residency",
source: &Source{CloneURL: "https://acme.ghe.com/acme/repo.git"},
want: "https://api.acme.ghe.com",
},
{
name: "non-github host",
source: &Source{CloneURL: "https://gitlab.com/acme/repo.git"},
Expand Down
19 changes: 19 additions & 0 deletions internal/install/github_host.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package install

import "strings"

func isGitHubLikeHost(host string) bool {
host = strings.ToLower(strings.TrimSpace(host))
return strings.Contains(host, "github") || strings.HasSuffix(host, ".ghe.com")
}

func gitHubAPIBaseForHost(host string) string {
host = strings.ToLower(strings.TrimSpace(host))
if host == "github.com" {
return "https://api.github.com"
}
if strings.HasSuffix(host, ".ghe.com") {
return "https://api." + host
}
return "https://" + host + "/api/v3"
}
6 changes: 3 additions & 3 deletions internal/install/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,9 @@ func normalizeCloneURL(u string) string {
u = strings.TrimSpace(u)
u = strings.TrimSuffix(u, ".git")
u = strings.TrimSuffix(u, "/")
// git@github.com:owner/repo → github.com/owner/repo
if strings.HasPrefix(u, "git@") {
u = strings.TrimPrefix(u, "git@")
// user@github.com:owner/repo → github.com/owner/repo
if at := strings.Index(u, "@"); at > 0 && !strings.Contains(u, "://") {
u = u[at+1:]
u = strings.Replace(u, ":", "/", 1)
}
// https://github.com/owner/repo → github.com/owner/repo
Expand Down
37 changes: 19 additions & 18 deletions internal/install/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ type Source struct {
// GitHub URL pattern: github.com/owner/repo[/path/to/subdir]
var githubPattern = regexp.MustCompile(`^(?:https?://)?github\.com/([^/]+)/([^/]+)(?:/(.+))?$`)

// Git SSH pattern: git@host:owner/repo[.git][//subdir]
var gitSSHPattern = regexp.MustCompile(`^git@([^:]+):([^/]+)/(.+?)(?:\.git)?(?://(.+))?$`)
// Git SSH pattern: user@host:owner/repo[.git][//subdir]
var gitSSHPattern = regexp.MustCompile(`^([^@:\s]+)@([^:\s]+):([^/]+)/(.+?)(?:\.git)?(?://(.+))?$`)

// Git HTTPS pattern: https://host/path (flexible path for GitLab subgroups)
var gitHTTPSPattern = regexp.MustCompile(`^(https?)://([^/]+)/(.+)$`)
Expand Down Expand Up @@ -179,7 +179,7 @@ func expandGitHubShorthand(input string) string {
if strings.HasPrefix(input, "github.com/") ||
strings.HasPrefix(input, "http://") ||
strings.HasPrefix(input, "https://") ||
strings.HasPrefix(input, "git@") ||
gitSSHPattern.MatchString(input) ||
strings.HasPrefix(input, "file://") ||
isLocalPath(input) {
return input
Expand Down Expand Up @@ -299,17 +299,18 @@ func trimSkillFileSuffix(path string, isBlob bool) (string, bool) {
}

func parseGitSSH(matches []string, source *Source) (*Source, error) {
// matches: [full, host, owner, repo, subdir]
host := matches[1]
owner := matches[2]
repo := strings.TrimSuffix(matches[3], ".git")
// matches: [full, user, host, owner, repo, subdir]
user := matches[1]
host := matches[2]
owner := matches[3]
repo := strings.TrimSuffix(matches[4], ".git")
subdir := ""
if len(matches) > 4 {
subdir = matches[4]
if len(matches) > 5 {
subdir = matches[5]
}

source.Type = SourceTypeGitSSH
source.CloneURL = fmt.Sprintf("git@%s:%s/%s.git", host, owner, repo)
source.CloneURL = fmt.Sprintf("%s@%s:%s/%s.git", user, host, owner, repo)

if subdir != "" {
source.Subdir = subdir
Expand Down Expand Up @@ -541,21 +542,21 @@ func (s *Source) gitHubOwnerRepo() (owner, repo string) {
return "", ""
}

// SSH clone URL: git@host:owner/repo.git
// SSH clone URL: user@host:owner/repo.git
if sshMatches := gitSSHPattern.FindStringSubmatch(cloneURL); sshMatches != nil {
host := strings.ToLower(strings.TrimSpace(sshMatches[1]))
if !strings.Contains(host, "github") {
host := strings.ToLower(strings.TrimSpace(sshMatches[2]))
if !isGitHubLikeHost(host) {
return "", ""
}
Comment on lines +547 to 550
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

For GitHub Enterprise Data Residency environments (which use *.ghe.com domains), the host will not contain the substring "github". As a result, gitHubOwnerRepo() will fail to extract the owner and repository name, returning empty strings. This will break any GitHub-specific integrations or metadata extraction for these sources.

Please update the host check to also recognize ghe.com domains. Note that you should also apply a similar update to the HTTPS parsing logic below (around line 559), although it is outside the current diff hunk.

Suggested change
host := strings.ToLower(strings.TrimSpace(sshMatches[2]))
if !strings.Contains(host, "github") {
return "", ""
}
host := strings.ToLower(strings.TrimSpace(sshMatches[2]))
if !strings.Contains(host, "github") && !strings.Contains(host, "ghe.com") {
return "", ""
}

return sshMatches[2], strings.TrimSuffix(sshMatches[3], ".git")
return sshMatches[3], strings.TrimSuffix(sshMatches[4], ".git")
}

u, err := url.Parse(cloneURL)
if err != nil {
return "", ""
}
host := strings.ToLower(u.Hostname())
if !strings.Contains(host, "github") {
if !isGitHubLikeHost(host) {
return "", ""
}

Expand Down Expand Up @@ -614,10 +615,10 @@ func (s *Source) TrackName() string {
}
}

// Try SSH format: git@host:owner/repo.git
// Try SSH format: user@host:owner/repo.git
if sshMatches := gitSSHPattern.FindStringSubmatch(s.Raw); sshMatches != nil {
owner := sshMatches[2]
repo := strings.TrimSuffix(sshMatches[3], ".git")
owner := sshMatches[3]
repo := strings.TrimSuffix(sshMatches[4], ".git")
// Replace / with - to handle subgroup paths (e.g., group/subgroup/repo)
return owner + "-" + strings.ReplaceAll(repo, "/", "-")
}
Expand Down
43 changes: 43 additions & 0 deletions internal/install/source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,18 @@ func TestParseSource_GitSSH(t *testing.T) {
wantCloneURL: "git@gitlab.com:user/repo.git",
wantName: "repo",
},
{
name: "custom ssh username",
input: "acme@acme.ghe.com:MyOrg/my-skills.git",
wantCloneURL: "acme@acme.ghe.com:MyOrg/my-skills.git",
wantName: "my-skills",
},
{
name: "custom ssh username without .git",
input: "acme@acme.ghe.com:MyOrg/my-skills",
wantCloneURL: "acme@acme.ghe.com:MyOrg/my-skills.git",
wantName: "my-skills",
},
{
name: "gitlab ssh nested subgroup",
input: "git@gitlab.example.com:org/subgroup/my-skills.git",
Expand Down Expand Up @@ -255,6 +267,13 @@ func TestParseSource_GitSSH(t *testing.T) {
wantSubdir: "pdf",
wantName: "pdf",
},
{
name: "custom ssh username with subpath",
input: "acme@acme.ghe.com:MyOrg/my-skills.git//agents/reviewer",
wantCloneURL: "acme@acme.ghe.com:MyOrg/my-skills.git",
wantSubdir: "agents/reviewer",
wantName: "reviewer",
},
}

for _, tt := range tests {
Expand Down Expand Up @@ -823,6 +842,13 @@ func TestParseSource_GitHubEnterprise(t *testing.T) {
wantCloneURL: "git@mycompany.github.com:team/skills.git",
wantName: "skills",
},
{
name: "GHE Data Residency SSH",
input: "acme@acme.ghe.com:MyOrg/my-skills.git",
wantType: SourceTypeGitSSH,
wantCloneURL: "acme@acme.ghe.com:MyOrg/my-skills.git",
wantName: "my-skills",
},
{
name: "GHE SSH with subdir",
input: "git@github.mycompany.com:org/repo.git//path/to/skill",
Expand Down Expand Up @@ -876,6 +902,11 @@ func TestParseSource_GitHubEnterprise_TrackName(t *testing.T) {
raw: "git@github.mycompany.com:org/skills.git",
want: "org-skills",
},
{
name: "GHE Data Residency SSH",
raw: "acme@acme.ghe.com:MyOrg/my-skills.git",
want: "MyOrg-my-skills",
},
}

for _, tt := range tests {
Expand Down Expand Up @@ -917,6 +948,18 @@ func TestSource_GitHubOwnerRepo(t *testing.T) {
wantOwner: "team",
wantRepo: "repo",
},
{
name: "ghe data residency https",
raw: "https://acme.ghe.com/MyOrg/my-skills/agents/reviewer",
wantOwner: "MyOrg",
wantRepo: "my-skills",
},
{
name: "ghe data residency ssh",
raw: "acme@acme.ghe.com:MyOrg/my-skills.git//agents/reviewer",
wantOwner: "MyOrg",
wantRepo: "my-skills",
},
{
name: "non-github host",
raw: "https://gitlab.com/team/repo",
Expand Down
Loading