From df97aa3a6f97fd8a9800b02f1b92a001b4e0b3d5 Mon Sep 17 00:00:00 2001 From: lorenzozane Date: Sat, 24 Oct 2026 11:30:43 +0800 Subject: [PATCH] fix(gitlab): recognize SSH-over-443 remotes (#30998) * fix(gitlab): recognize SSH-over-443 remotes * fix(gitlab): canonicalize SSH-over-443 project and review URLs Reuse the existing GitLab transport-host normalizer in project identity, hosted links, and manual merge-request links. Cover alias variants, lookalikes, nested groups, and canonical encoded links. Fixes #30876 Co-authored-by: lorenzozanee * test(gitlab): cover alias identity consumers --------- Co-authored-by: Neil Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> --- src/cli/index-worktree-gitlab-links.test.ts | 11 ++- src/main/git/hosted-remote-url.test.ts | 24 +++++++ src/main/git/hosted-remote-url.ts | 4 +- src/main/gitlab/project-ref-parser.test.ts | 27 ++++++++ src/main/gitlab/project-ref-parser.ts | 5 +- .../source-control-manual-review-url.test.ts | 42 ++++++++++++ .../source-control/review/remote-repo.ts | 20 ++++-- .../review/use-hosted-review-state.test.tsx | 67 +++++++++++++++++++ 8 files changed, 187 insertions(+), 13 deletions(-) diff --git a/src/cli/index-worktree-gitlab-links.test.ts b/src/cli/index-worktree-gitlab-links.test.ts index ecd76f00005..2a16d5a754d 100644 --- a/src/cli/index-worktree-gitlab-links.test.ts +++ b/src/cli/index-worktree-gitlab-links.test.ts @@ -170,11 +170,14 @@ describe('GitLab flag project identity', () => { }) } - it('accepts an scp remote without guessing a web port', async () => { + it.each([ + 'git@gitlab.com:group/project.git', + 'ssh://git@altssh.gitlab.com:443/group/project.git' + ])('matches GitLab issue and MR URLs against stored remote %s', async (remoteUrl) => { queueFixtures( callMock, okFixture('req_repo', { - repo: { id: 'repo', gitRemoteIdentity: { remoteUrl: 'git@gitlab.com:group/project.git' } } + repo: { id: 'repo', gitRemoteIdentity: { remoteUrl } } }), okFixture('req_create', { worktree: buildWorktree('/tmp/repo/child', 'linked-task'), @@ -192,6 +195,8 @@ describe('GitLab flag project identity', () => { '--name', 'linked-task', '--no-parent', + '--gitlab-issue', + 'https://gitlab.com/group/project/-/issues/53', '--gitlab-mr', 'https://gitlab.com/group/project/-/merge_requests/77/diffs', '--json' @@ -201,7 +206,7 @@ describe('GitLab flag project identity', () => { expect(callMock).toHaveBeenNthCalledWith( 2, 'worktree.create', - expect.objectContaining({ linkedGitLabMR: 77 }) + expect.objectContaining({ linkedGitLabIssue: 53, linkedGitLabMR: 77 }) ) }) }) diff --git a/src/main/git/hosted-remote-url.test.ts b/src/main/git/hosted-remote-url.test.ts index 7260f741117..2dec525812b 100644 --- a/src/main/git/hosted-remote-url.test.ts +++ b/src/main/git/hosted-remote-url.test.ts @@ -27,6 +27,16 @@ describe('hosted remote URLs', () => { path: 'Org/Repo', provider: 'github' }) + expect(parseHostedRemote('ssh://git@altssh.gitlab.com:443/team/project.git')).toEqual({ + host: 'gitlab.com', + path: 'team/project', + provider: 'gitlab' + }) + expect(parseHostedRemote('git@ALTSSH.GITLAB.COM:team/subgroup/project.git')).toEqual({ + host: 'gitlab.com', + path: 'team/subgroup/project', + provider: 'gitlab' + }) expect(parseHostedRemote('github:Org/Repo')).toEqual({ host: 'github.com', path: 'Org/Repo', @@ -48,6 +58,14 @@ describe('hosted remote URLs', () => { }) it('builds file URLs with encoded branches and paths', () => { + expect( + buildHostedRemoteFileUrl( + 'ssh://git@altssh.gitlab.com:443/team/project.git', + 'src/a file.ts', + 'feature/x', + 5 + ) + ).toBe('https://gitlab.com/team/project/-/blob/feature%2Fx/src/a%20file.ts#L5') expect( buildHostedRemoteFileUrl('git@github.com:Org/Repo.git', 'src/a file.ts', 'feature/x', 42) ).toBe('https://github.com/Org/Repo/blob/feature%2Fx/src/a%20file.ts#L42') @@ -87,6 +105,9 @@ describe('hosted remote URLs', () => { expect(buildHostedRemoteCommitUrl('git@bitbucket.org:team/repo.git', sha)).toBe( `https://bitbucket.org/team/repo/commits/${sha}` ) + expect(buildHostedRemoteCommitUrl('git@ALTSSH.GITLAB.COM:team/subgroup/project.git', sha)).toBe( + `https://gitlab.com/team/subgroup/project/-/commit/${sha}` + ) }) it('returns null for unsupported commit remotes or missing sha', () => { @@ -103,5 +124,8 @@ describe('hosted remote URLs', () => { expect(parseHostedRemote('git@example.com:team/repo.git')).toBeNull() expect(parseHostedRemote('git@github.com:repo.git')).toBeNull() expect(parseHostedRemote('ftp://github.com/Org/Repo.git')).toBeNull() + expect(parseHostedRemote('git@altssh.gitlab.com.example.com:team/repo.git')).toBeNull() + expect(parseHostedRemote('ssh://git@altssh.gitlab.com.example.com/team/repo.git')).toBeNull() + expect(parseHostedRemote('git@notaltssh.gitlab.com:team/repo.git')).toBeNull() }) }) diff --git a/src/main/git/hosted-remote-url.ts b/src/main/git/hosted-remote-url.ts index 22b911e8a7a..349d911cf5c 100644 --- a/src/main/git/hosted-remote-url.ts +++ b/src/main/git/hosted-remote-url.ts @@ -1,3 +1,5 @@ +import { normalizeGitLabRemoteHost } from '../../shared/git-remote-host-alias' + // Scoped local fork of the hosted-git-info@9.0.3 behavior Orca used. // Keep only the provider parsing and file/commit URL construction needed by source links. type HostedRemoteProvider = 'github' | 'gitlab' | 'bitbucket' @@ -20,7 +22,7 @@ const shorthandHosts: Record { }) }) + it.each([ + 'ssh://git@altssh.gitlab.com:443/team/project.git', + 'git@altssh.gitlab.com:team/project.git', + 'ssh://git@ALTSSH.GITLAB.COM:443/team/project.git', + 'git@ALTSSH.GITLAB.COM:team/project.git', + 'git+ssh://git@altssh.gitlab.com:443/team/project.git' + ])('parses the SSH alias as the canonical GitLab host: %s', (remote) => { + expect(parseGitLabProjectRef(remote)).toEqual({ + host: 'gitlab.com', + path: 'team/project' + }) + }) + + it.each([ + 'ssh://git@altssh.gitlab.com.example.com:443/team/project.git', + 'git@altssh.gitlab.com.example.com:team/project.git', + 'git@notaltssh.gitlab.com:team/project.git' + ])('does not trust an SSH alias lookalike: %s', (remote) => { + expect(parseGitLabProjectRef(remote)).toBeNull() + }) + it('preserves nested group paths', () => { expect(parseGitLabProjectRef('git@gitlab.com:group/subgroup/project.git')).toEqual({ host: 'gitlab.com', @@ -111,6 +132,12 @@ describe('gitlab project ref parsing', () => { }) describe('gitlab remote project ref candidates', () => { + it('canonicalizes the SSH alias before resolving auth for a candidate', () => { + expect( + parseRemoteProjectRefCandidate('ssh://git@altssh.gitlab.com:443/team/subgroup/project.git') + ).toEqual({ host: 'gitlab.com', path: 'team/subgroup/project' }) + }) + it('extracts self-hosted candidates before the host is trusted', () => { expect(parseRemoteProjectRefCandidate('git@gitlab.internal:team/orca.git')).toEqual({ host: 'gitlab.internal', diff --git a/src/main/gitlab/project-ref-parser.ts b/src/main/gitlab/project-ref-parser.ts index faaa69b4fa5..d131e0acb3a 100644 --- a/src/main/gitlab/project-ref-parser.ts +++ b/src/main/gitlab/project-ref-parser.ts @@ -1,4 +1,5 @@ import type { GitLabProjectRef } from '../../shared/gitlab-types' +import { normalizeGitLabRemoteHost } from '../../shared/git-remote-host-alias' export type ProjectRef = GitLabProjectRef @@ -27,7 +28,7 @@ function hostIdentityFromUrl(url: URL): string { } function makeProjectRefForTrustedHost(host: string, path: string): ProjectRef | null { - const normalizedHost = normalizeGitLabHost(host) + const normalizedHost = normalizeGitLabRemoteHost(host) const normalizedPath = stripGitSuffix(path.replace(/^\/+/, '')).trim() if (!normalizedPath.includes('/')) { return null @@ -51,7 +52,7 @@ function makeProjectRef( path: string, knownHosts: readonly string[] ): ProjectRef | null { - const normalizedHost = normalizeGitLabHost(host) + const normalizedHost = normalizeGitLabRemoteHost(host) const normalizedKnownHosts = knownHosts.map(normalizeGitLabHost) if (!normalizedKnownHosts.some((knownHost) => knownHostMatches(normalizedHost, knownHost))) { return null diff --git a/src/renderer/src/components/right-sidebar/source-control-manual-review-url.test.ts b/src/renderer/src/components/right-sidebar/source-control-manual-review-url.test.ts index 1f6749865fb..5a96f232547 100644 --- a/src/renderer/src/components/right-sidebar/source-control-manual-review-url.test.ts +++ b/src/renderer/src/components/right-sidebar/source-control-manual-review-url.test.ts @@ -4,6 +4,7 @@ import { buildSourceControlManualReviewUrlFromContext, resolveSourceControlManualReviewProvider } from './source-control/review/manual-review-url' +import { parseRemoteRepo } from './source-control/review/remote-repo' describe('resolveSourceControlManualReviewProvider', () => { it('prefers hosted review and creation providers over linked review hints', () => { @@ -78,6 +79,47 @@ describe('buildSourceControlManualReviewUrlFromContext', () => { }) describe('buildSourceControlManualReviewUrl', () => { + it.each([ + 'ssh://git@altssh.gitlab.com:443/team/project.git', + 'git@altssh.gitlab.com:team/project.git', + 'ssh://git@ALTSSH.GITLAB.COM:443/team/project.git', + 'git@ALTSSH.GITLAB.COM:team/project.git' + ])('canonicalizes GitLab SSH-over-443 web identity: %s', (remote) => { + expect(parseRemoteRepo(remote)).toEqual({ + provider: 'gitlab', + path: 'team/project', + webBaseUrl: 'https://gitlab.com/team/project' + }) + for (const provider of [undefined, 'gitlab'] as const) { + expect( + buildSourceControlManualReviewUrl({ + baseRef: 'main', + branchName: 'feature/x', + repoRemoteName: 'origin', + repoRemoteUrl: remote, + provider, + upstreamName: 'origin/feature/x' + }) + ).toBe( + 'https://gitlab.com/team/project/-/merge_requests/new?merge_request%5Bsource_branch%5D=feature%2Fx&merge_request%5Btarget_branch%5D=main' + ) + } + }) + + it('does not infer GitLab or rewrite the web origin of an alias lookalike', () => { + const remote = 'ssh://git@altssh.gitlab.com.example.com:443/team/project.git' + expect(parseRemoteRepo(remote)).toEqual({ + provider: null, + path: 'team/project', + webBaseUrl: 'https://altssh.gitlab.com.example.com/team/project' + }) + expect(parseRemoteRepo(remote, 'gitlab')).toEqual({ + provider: 'gitlab', + path: 'team/project', + webBaseUrl: 'https://altssh.gitlab.com.example.com/team/project' + }) + }) + it('builds a GitHub compare URL for the current remote branch', () => { expect( buildSourceControlManualReviewUrl({ diff --git a/src/renderer/src/components/right-sidebar/source-control/review/remote-repo.ts b/src/renderer/src/components/right-sidebar/source-control/review/remote-repo.ts index 7a9761816c4..65e978294db 100644 --- a/src/renderer/src/components/right-sidebar/source-control/review/remote-repo.ts +++ b/src/renderer/src/components/right-sidebar/source-control/review/remote-repo.ts @@ -1,4 +1,5 @@ import type { HostedReviewProvider } from '../../../../../../shared/hosted-review' +import { normalizeGitLabRemoteHost } from '../../../../../../shared/git-remote-host-alias' export type ManualReviewProvider = Exclude @@ -47,7 +48,7 @@ function cleanPath(path: string): string | null { } function providerForHost(host: string): ManualReviewProvider | null { - const normalized = host.toLowerCase() + const normalized = normalizeGitLabRemoteHost(host) if (normalized === 'github.com' || normalized === 'ssh.github.com') { return 'github' } @@ -153,7 +154,7 @@ export function parseRemoteRepo( ? trimmed.match(/^(?:[^@/:]+@)?([^:\s/]+):([^\s]+?)(?:\.git)?$/) : null if (scpLike) { - const host = scpLike[1].toLowerCase() + const host = normalizeGitLabRemoteHost(scpLike[1].toLowerCase()) const path = cleanPath(scpLike[2]) if (!path) { return null @@ -176,14 +177,19 @@ export function parseRemoteRepo( if (!path) { return null } - const host = url.hostname.toLowerCase() + const remoteHost = url.hostname.toLowerCase() + const host = normalizeGitLabRemoteHost(remoteHost) const hintedProvider = normalizeProvider(providerHint) const inferredProvider = providerForHost(host) const provider = hintedProvider ?? inferredProvider - const webOrigin = - host === 'ssh.github.com' - ? 'https://github.com' - : buildWebOrigin(protocol, url.host, url.hostname).replace(/\/+$/, '') + let webOrigin: string + if (host === 'ssh.github.com') { + webOrigin = 'https://github.com' + } else if (remoteHost === 'altssh.gitlab.com') { + webOrigin = 'https://gitlab.com' + } else { + webOrigin = buildWebOrigin(protocol, url.host, url.hostname).replace(/\/+$/, '') + } return { provider, path, diff --git a/src/renderer/src/components/right-sidebar/source-control/review/use-hosted-review-state.test.tsx b/src/renderer/src/components/right-sidebar/source-control/review/use-hosted-review-state.test.tsx index 17d270e3570..07e101f1a38 100644 --- a/src/renderer/src/components/right-sidebar/source-control/review/use-hosted-review-state.test.tsx +++ b/src/renderer/src/components/right-sidebar/source-control/review/use-hosted-review-state.test.tsx @@ -5,6 +5,8 @@ import { describe, expect, it } from 'vitest' import type { HostedReviewInfo } from '../../../../../../shared/hosted-review' import type { PRInfo } from '../../../../../../shared/github/pull-request-types' import { useSourceControlHostedReviewState } from './use-hosted-review-state' +import { useSourceControlHostedReviewProviderHint } from './use-hosted-review-provider-hint' +import { resolveCreatePrHeaderAction } from './primary-create-pr-intent-action' function makePR(number = 42): PRInfo { return { @@ -44,6 +46,71 @@ function renderState({ } describe('useSourceControlHostedReviewState', () => { + it('uses MR header copy while eligibility loads for a GitLab SSH-over-443 remote', () => { + const { result } = renderHook(() => { + const state = useSourceControlHostedReviewState({ + activePrFromQueue: null, + activeRepoId: 'repo-1', + activeWorktreeId: 'wt-1', + branchName: 'feature', + hostedReviewCacheKey: 'cache-key', + hostedReviewEntryData: null, + linkedPR: null, + suppressedGitHubPR: null + }) + return useSourceControlHostedReviewProviderHint({ + activeRepo: { + id: 'repo-1', + path: '/repo', + displayName: 'repo', + badgeColor: '', + addedAt: 0, + gitRemoteIdentity: { + remoteUrl: 'ssh://git@altssh.gitlab.com:443/group/project.git', + canonicalKey: 'gitlab.com/group/project', + remoteName: 'origin' + } + }, + activeRepoId: 'repo-1', + activeWorktreeId: 'wt-1', + branchName: 'feature', + fallbackGitHubPRNumber: null, + hostedReview: state.hostedReview, + hostedReviewCreation: state.hostedReviewCreation, + hostedReviewCreationProviderHintRef: state.hostedReviewCreationProviderHintRef, + hostedReviewCreationRequestState: { + repoId: 'repo-1', + worktreeId: 'wt-1', + branch: 'feature', + status: 'loading' + }, + isBranchVisible: true, + isFolder: false, + linkedAzureDevOpsPR: null, + linkedBitbucketPR: null, + linkedGitHubPR: null, + linkedGitLabMR: null, + linkedGiteaPR: null + }) + }) + + expect( + resolveCreatePrHeaderAction({ + stagedCount: 0, + hasUnstagedChanges: false, + hasStageableChanges: false, + hasPartiallyStagedChanges: false, + hasMessage: false, + hasUnresolvedConflicts: false, + isCommitting: false, + isRemoteOperationActive: false, + upstreamStatus: undefined, + hostedReviewCreation: result.current.hostedReviewCreationForHeader, + isHostedReviewCreationLoading: result.current.isHostedReviewCreationLoading + }) + ).toMatchObject({ kind: 'create_pr', label: 'Create MR', disabled: true }) + }) + it('hides a matching suppressed PR and blocks duplicate creation', () => { const state = renderState({ suppressedGitHubPR: 42 })