Skip to content

Support custom SSH usernames for install sources#178

Merged
runkids merged 2 commits into
runkids:mainfrom
PeterTianbuhan:codex/ghe-ssh-username
May 31, 2026
Merged

Support custom SSH usernames for install sources#178
runkids merged 2 commits into
runkids:mainfrom
PeterTianbuhan:codex/ghe-ssh-username

Conversation

@PeterTianbuhan
Copy link
Copy Markdown
Contributor

Implemented parsing for SSH source URLs that use a custom username, such as GitHub Enterprise Data Residency remotes like acme@acme.ghe.com:Org/repo.git. The parser now preserves the SSH username in the clone URL, skips GitHub shorthand expansion for these inputs, and normalizes custom-username SSH URLs for conflict detection.

This fixes installs from GHE Data Residency environments where the SSH username is not git.

Fixes #176

Tests:

  • go test ./internal/install -run 'TestParseSource_GitSSH|TestParseSource_GitHubEnterprise|TestParseSource_GitHubEnterprise_TrackName|TestNormalizeCloneURL'
  • ./scripts/test.sh --unit --quiet
  • ./scripts/test.sh --quiet

Copilot AI review requested due to automatic review settings May 31, 2026 12:14
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the SSH URL parsing and normalization logic to support custom SSH usernames (e.g., for GitHub Enterprise Data Residency environments using *.ghe.com domains) instead of hardcoding git@. This includes updating regex patterns, normalization logic, and parsing functions, along with adding corresponding unit tests. Feedback on the changes highlights an issue in gitHubOwnerRepo(), where the host check only looks for the substring "github". This will fail for GHE Data Residency domains (*.ghe.com), so the check should be updated to also recognize ghe.com.

Comment on lines +547 to 550
host := strings.ToLower(strings.TrimSpace(sshMatches[2]))
if !strings.Contains(host, "github") {
return "", ""
}
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 "", ""
}

@TIR44
Copy link
Copy Markdown
Contributor

TIR44 commented May 31, 2026

Checked this path locally because it touches the install source parser and the PR is currently behind main. I did not find a blocker.

What I verified:

  • custom SSH usernames keep the intended clone URL while conflict normalization strips the SSH username for repo identity matching
  • *.ghe.com now flows through platform detection, GitHubOwner / GitHubRepo, and GitHub API base selection
  • git merge-tree 549d466f8f1b4903e45cae2816916e80c8601c65 HEAD origin/main did not report a textual conflict against current origin/main

Local validation on 5fd487b8:

  • go test ./internal/install -run 'TestParseSource_GitSSH|TestParseSource_GitHubEnterprise|TestParseSource_GitHubEnterprise_TrackName|TestNormalizeCloneURL|TestDetectPlatform|TestGitHubAPIBase|TestSource_GitHubOwnerRepo' -count=1
  • go test ./internal/install -count=1
  • ./scripts/test.sh --unit --quiet

The only operational note I see is that the branch is behind main, but the install-package changes looked clean in the merge check above.

@runkids runkids merged commit d35ed81 into runkids:main May 31, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SSH install fails for GitHub Enterprise Data Residency (custom SSH username)

3 participants