From 11cbd8cfeef79749090dbee4ba61f0f76e9e07f1 Mon Sep 17 00:00:00 2001 From: maslin Date: Sun, 16 Aug 2026 16:11:37 +0700 Subject: [PATCH 1/2] fix(vcs): resolve SSH aliases before classifying GitHub hosts Undotted remotes like git@github-personal:owner/repo were treated as GitHub Self-Hosted, so the pull-request pane invoked gh against a fake host. Detection now requires a real hostname before calling a remote self-hosted, and the server rewrites SSH aliases via ssh -G HostName for API/browser host while git auth still uses the alias. --- apps/server/package.json | 1 + .../src/project/RepositoryIdentityResolver.ts | 44 +++++- .../pullRequest/PullRequestService.test.ts | 2 + .../src/pullRequest/PullRequestService.ts | 139 ++++++++++++------ .../SourceControlDiscovery.test.ts | 1 + .../SourceControlProviderRegistry.test.ts | 1 + .../SourceControlProviderRegistry.ts | 24 ++- .../resolveGitRemoteForSourceControl.test.ts | 69 +++++++++ .../resolveGitRemoteForSourceControl.ts | 100 +++++++++++++ docs/user/source-control.md | 2 + packages/shared/src/sourceControl.test.ts | 56 +++++++ packages/shared/src/sourceControl.ts | 49 +++++- pnpm-lock.yaml | 3 + 13 files changed, 432 insertions(+), 59 deletions(-) create mode 100644 apps/server/src/sourceControl/resolveGitRemoteForSourceControl.test.ts create mode 100644 apps/server/src/sourceControl/resolveGitRemoteForSourceControl.ts diff --git a/apps/server/package.json b/apps/server/package.json index eb4dc7dd35ec..719b0679699c 100644 --- a/apps/server/package.json +++ b/apps/server/package.json @@ -39,6 +39,7 @@ "@effect/vitest": "catalog:", "@t3tools/contracts": "workspace:*", "@t3tools/shared": "workspace:*", + "@t3tools/ssh": "workspace:*", "@t3tools/tailscale": "workspace:*", "@t3tools/web": "workspace:*", "@types/bun": "1.3.14", diff --git a/apps/server/src/project/RepositoryIdentityResolver.ts b/apps/server/src/project/RepositoryIdentityResolver.ts index 50608e7704c7..0b02409d827e 100644 --- a/apps/server/src/project/RepositoryIdentityResolver.ts +++ b/apps/server/src/project/RepositoryIdentityResolver.ts @@ -3,6 +3,7 @@ import { detectSourceControlProviderFromGitRemoteUrl, normalizeGitRemoteUrl, } from "@t3tools/shared/git"; +import { parseSshResolveOutput } from "@t3tools/ssh/command"; import * as Cache from "effect/Cache"; import * as Context from "effect/Context"; import * as Duration from "effect/Duration"; @@ -11,6 +12,7 @@ import * as Exit from "effect/Exit"; import * as Layer from "effect/Layer"; import * as ProcessRunner from "../processRunner.ts"; +import { resolveGitRemoteForSourceControl } from "../sourceControl/resolveGitRemoteForSourceControl.ts"; const DEFAULT_REPOSITORY_IDENTITY_CACHE_CAPACITY = 512; const DEFAULT_POSITIVE_CACHE_TTL = Duration.minutes(1); @@ -60,13 +62,39 @@ function pickPrimaryRemote( return remoteName && remoteUrl ? { remoteName, remoteUrl } : null; } +// This layer is provided in many tests without NodeServices. Resolve HostName +// through ProcessRunner so SSH Effect services do not leak onto the service. +function resolveSshHostnameWithProcessRunner( + processRunner: ProcessRunner.ProcessRunner["Service"], +): (alias: string) => Effect.Effect { + return (alias) => + processRunner + .run({ + command: "ssh", + args: ["-G", alias], + timeout: Duration.seconds(5), + timeoutBehavior: "timedOutResult", + }) + .pipe( + Effect.map((result) => { + if (result.timedOut || result.code !== 0) { + return null; + } + const hostname = parseSshResolveOutput(alias, result.stdout).hostname.trim(); + return hostname.length > 0 ? hostname : null; + }), + Effect.orElseSucceed(() => null), + ); +} + function buildRepositoryIdentity(input: { readonly remoteName: string; readonly remoteUrl: string; + readonly detectionUrl: string; readonly rootPath: string; }): RepositoryIdentity { - const canonicalKey = normalizeGitRemoteUrl(input.remoteUrl); - const sourceControlProvider = detectSourceControlProviderFromGitRemoteUrl(input.remoteUrl); + const canonicalKey = normalizeGitRemoteUrl(input.detectionUrl); + const sourceControlProvider = detectSourceControlProviderFromGitRemoteUrl(input.detectionUrl); const repositoryPath = canonicalKey.split("/").slice(1).join("/"); const repositoryPathSegments = repositoryPath.split("/").filter((segment) => segment.length > 0); const [owner] = repositoryPathSegments; @@ -118,6 +146,7 @@ const resolveRepositoryIdentityFromCacheKey = Effect.fn( "RepositoryIdentityResolver.resolveFromCacheKey", )(function* ( cacheKey: string, + resolveDetectionUrl: (remoteUrl: string) => Effect.Effect, ): Effect.fn.Return { const processRunner = yield* ProcessRunner.ProcessRunner; const remoteResult = yield* processRunner @@ -132,17 +161,24 @@ const resolveRepositoryIdentityFromCacheKey = Effect.fn( } const remote = pickPrimaryRemote(parseRemoteFetchUrls(remoteResult.value.stdout)); - return remote ? buildRepositoryIdentity({ ...remote, rootPath: cacheKey }) : null; + if (!remote) { + return null; + } + + const detectionUrl = yield* resolveDetectionUrl(remote.remoteUrl); + return buildRepositoryIdentity({ ...remote, detectionUrl, rootPath: cacheKey }); }); export const make = Effect.fn("RepositoryIdentityResolver.make")(function* ( options: RepositoryIdentityResolverOptions = {}, ) { const processRunner = yield* ProcessRunner.ProcessRunner; + const resolveDetectionUrl = (remoteUrl: string) => + resolveGitRemoteForSourceControl(remoteUrl, resolveSshHostnameWithProcessRunner(processRunner)); const repositoryIdentityCache = yield* Cache.makeWith( (cacheKey) => - resolveRepositoryIdentityFromCacheKey(cacheKey).pipe( + resolveRepositoryIdentityFromCacheKey(cacheKey, resolveDetectionUrl).pipe( Effect.provideService(ProcessRunner.ProcessRunner, processRunner), ), { diff --git a/apps/server/src/pullRequest/PullRequestService.test.ts b/apps/server/src/pullRequest/PullRequestService.test.ts index 456a5023b16d..8046f85c44e9 100644 --- a/apps/server/src/pullRequest/PullRequestService.test.ts +++ b/apps/server/src/pullRequest/PullRequestService.test.ts @@ -1,4 +1,5 @@ import { assert, it } from "@effect/vitest"; +import * as NodeServices from "@effect/platform-node/NodeServices"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; import type { @@ -155,6 +156,7 @@ function makeService(input: { return PullRequestService.make.pipe( Effect.provide( Layer.mergeAll( + NodeServices.layer, Layer.succeed(PullRequestProviderRegistry, fromProviders(input.providers)), Layer.mock(SourceControlProviderRegistry.SourceControlProviderRegistry)({ resolveHandle: diff --git a/apps/server/src/pullRequest/PullRequestService.ts b/apps/server/src/pullRequest/PullRequestService.ts index f12f72bdafac..b0e1ae78d17e 100644 --- a/apps/server/src/pullRequest/PullRequestService.ts +++ b/apps/server/src/pullRequest/PullRequestService.ts @@ -47,6 +47,10 @@ import { import { detectSourceControlProviderFromRemoteUrl } from "@t3tools/shared/sourceControl"; import * as ProjectionSnapshotQuery from "../orchestration/Services/ProjectionSnapshotQuery.ts"; +import { + resolveGitRemoteForSourceControl, + type ResolveGitRemoteServices, +} from "../sourceControl/resolveGitRemoteForSourceControl.ts"; import * as SourceControlProviderRegistry from "../sourceControl/SourceControlProviderRegistry.ts"; import { type ProviderChangeRequest, @@ -390,10 +394,27 @@ export function repositoryIdentityOf(project: OrchestrationProjectShell): string return identity.owner && identity.name ? `${identity.owner}/${identity.name}` : null; } +function hostnameFromProviderBaseUrl(baseUrl: string): string | null { + try { + const hostname = new URL(baseUrl).hostname.toLowerCase(); + return hostname.length > 0 ? hostname : null; + } catch { + return null; + } +} + export const make = Effect.gen(function* () { const registry = yield* PullRequestProviderRegistry; const projections = yield* ProjectionSnapshotQuery.ProjectionSnapshotQuery; const sourceControlProviders = yield* SourceControlProviderRegistry.SourceControlProviderRegistry; + const sshContext = yield* Effect.context(); + const resolveDetectionUrl = (remoteUrl: string) => + resolveGitRemoteForSourceControl(remoteUrl).pipe(Effect.provideContext(sshContext)); + + const detectProviderForRemoteUrl = (remoteUrl: string) => + resolveDetectionUrl(remoteUrl).pipe( + Effect.map((detectionUrl) => detectSourceControlProviderFromRemoteUrl(detectionUrl)), + ); const refineUnknownProjectKinds = ( projects: ReadonlyArray, @@ -405,52 +426,62 @@ export const make = Effect.gen(function* () { readonly remoteName: string; readonly remoteUrl: string; }; - const refinements = new Map(); - for (const project of projects) { - if (filter.projectId !== undefined && project.id !== filter.projectId) continue; - const identity = project.repositoryIdentity; - if (identity?.provider !== "unknown" || repositoryIdentityOf(project) === null) continue; - const host = pullRequestHostOf(identity, "unknown"); - // A legacy identity has no canonical host until its provider is refined, so it must reach - // the refinement before a host filter can decide whether it belongs in the result. - if (filter.host !== undefined && host !== "unknown" && host !== filter.host.toLowerCase()) { - continue; - } - const { remoteName, remoteUrl } = identity.locator; - const provider = detectSourceControlProviderFromRemoteUrl(remoteUrl); - if (provider !== null) { - const candidates = refinements.get(provider.baseUrl); - const candidate = { project, provider, remoteName, remoteUrl }; - if (candidates === undefined) refinements.set(provider.baseUrl, [candidate]); - else candidates.push(candidate); + + return Effect.gen(function* () { + const refinements = new Map(); + const detectedByRemoteUrl = new Map(); + for (const project of projects) { + if (filter.projectId !== undefined && project.id !== filter.projectId) continue; + const identity = project.repositoryIdentity; + if (identity?.provider !== "unknown" || repositoryIdentityOf(project) === null) continue; + const { remoteName, remoteUrl } = identity.locator; + let provider = detectedByRemoteUrl.get(remoteUrl); + if (provider === undefined) { + provider = yield* detectProviderForRemoteUrl(remoteUrl); + detectedByRemoteUrl.set(remoteUrl, provider); + } + const host = pullRequestHostOf(identity, "unknown"); + // A legacy identity has no canonical host until its provider is refined, so it must reach + // the refinement before a host filter can decide whether it belongs in the result. + if (filter.host !== undefined && host !== "unknown" && host !== filter.host.toLowerCase()) { + continue; + } + if (provider !== null) { + const candidates = refinements.get(provider.baseUrl); + const candidate = { project, provider, remoteName, remoteUrl }; + if (candidates === undefined) refinements.set(provider.baseUrl, [candidate]); + else candidates.push(candidate); + } } - } - return Effect.forEach( - refinements, - ([baseUrl, candidates]) => - Effect.firstSuccessOf( - candidates.map(({ project, provider, remoteName, remoteUrl }) => - Effect.suspend(() => - sourceControlProviders.resolveHandle({ - cwd: project.workspaceRoot, - context: { provider, remoteName, remoteUrl }, - }), - ).pipe( - Effect.flatMap((handle) => { - const kind = handle.context?.provider.kind; - return kind === undefined || kind === "unknown" - ? Effect.fail(undefined) - : Effect.succeed(kind); - }), + const refinedKinds = yield* Effect.forEach( + refinements, + ([baseUrl, candidates]) => + Effect.firstSuccessOf( + candidates.map(({ project, provider, remoteName, remoteUrl }) => + Effect.suspend(() => + sourceControlProviders.resolveHandle({ + cwd: project.workspaceRoot, + context: { provider, remoteName, remoteUrl }, + }), + ).pipe( + Effect.flatMap((handle) => { + const kind = handle.context?.provider.kind; + return kind === undefined || kind === "unknown" + ? Effect.fail(undefined) + : Effect.succeed(kind); + }), + ), ), + ).pipe( + Effect.map((kind) => [baseUrl, kind] as const), + Effect.orElseSucceed(() => [baseUrl, "unknown"] as const), ), - ).pipe( - Effect.map((kind) => [baseUrl, kind] as const), - Effect.orElseSucceed(() => [baseUrl, "unknown"] as const), - ), - { concurrency: REPOSITORY_CONCURRENCY }, - ).pipe(Effect.map((resolved) => new Map(resolved))); + { concurrency: REPOSITORY_CONCURRENCY }, + ).pipe(Effect.map((resolved) => new Map(resolved))); + + return { refinedKinds, detectedByRemoteUrl }; + }); }; const listWorkspaceProjects = ( @@ -467,10 +498,14 @@ export const make = Effect.gen(function* () { ), Effect.flatMap((snapshot) => refineUnknownProjectKinds(snapshot.projects, filter).pipe( - Effect.map((refinedKinds) => ({ refinedKinds, snapshot })), + Effect.map(({ refinedKinds, detectedByRemoteUrl }) => ({ + refinedKinds, + detectedByRemoteUrl, + snapshot, + })), ), ), - Effect.map(({ refinedKinds, snapshot }) => { + Effect.map(({ refinedKinds, detectedByRemoteUrl, snapshot }) => { const supported: SupportedProject[] = []; const unimplemented = new Map< string, @@ -488,11 +523,21 @@ export const make = Effect.gen(function* () { // Worktrees of one repository are separate projects; reading the remote once keeps // the page from repeating every change request per local checkout. The host is part // of the key, so the same `owner/repo` on two hosts stays two repositories. + const detected = + kind === "unknown" + ? (detectedByRemoteUrl.get(identity.locator.remoteUrl) ?? null) + : null; if (kind === "unknown") { - const provider = detectSourceControlProviderFromRemoteUrl(identity.locator.remoteUrl); - kind = provider === null ? kind : (refinedKinds.get(provider.baseUrl) ?? kind); + kind = + detected === null + ? kind + : (refinedKinds.get(detected.baseUrl) ?? + (detected.kind === "unknown" ? kind : detected.kind)); } - const host = pullRequestHostOf(identity, kind); + const host = + detected !== null && detected.kind !== "unknown" + ? (hostnameFromProviderBaseUrl(detected.baseUrl) ?? pullRequestHostOf(identity, kind)) + : pullRequestHostOf(identity, kind); if (filter.host !== undefined && host !== filter.host.toLowerCase()) continue; const api = registry.get(kind); // Recorded before the de-duplication below, so the viewer lookup keeps the alternates diff --git a/apps/server/src/sourceControl/SourceControlDiscovery.test.ts b/apps/server/src/sourceControl/SourceControlDiscovery.test.ts index 9e4702af04cd..fbc5137b15c2 100644 --- a/apps/server/src/sourceControl/SourceControlDiscovery.test.ts +++ b/apps/server/src/sourceControl/SourceControlDiscovery.test.ts @@ -23,6 +23,7 @@ const sourceControlProviderRegistryTestLayer = (input: { SourceControlProviderRegistry.layer.pipe( Layer.provide( Layer.mergeAll( + NodeServices.layer, ServerConfig.layerTest(process.cwd(), { prefix: "t3-source-control-registry-test-", }).pipe(Layer.provide(NodeServices.layer)), diff --git a/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts b/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts index 54038502bfde..f49218f51759 100644 --- a/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts +++ b/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts @@ -88,6 +88,7 @@ function makeRegistry(input: { Layer.mergeAll( registryLayer, processLayer, + NodeServices.layer, Layer.mock(AzureDevOpsCli.AzureDevOpsCli)({}), Layer.mock(BitbucketApi.BitbucketApi)({}), Layer.mock(GitHubCli.GitHubCli)({}), diff --git a/apps/server/src/sourceControl/SourceControlProviderRegistry.ts b/apps/server/src/sourceControl/SourceControlProviderRegistry.ts index 9fe089a4184c..52048be443cb 100644 --- a/apps/server/src/sourceControl/SourceControlProviderRegistry.ts +++ b/apps/server/src/sourceControl/SourceControlProviderRegistry.ts @@ -21,6 +21,10 @@ import { refineUnknownRemoteProvider, type SourceControlProviderDiscoverySpec, } from "./SourceControlProviderDiscovery.ts"; +import { + resolveGitRemoteForSourceControl, + type ResolveGitRemoteServices, +} from "./resolveGitRemoteForSourceControl.ts"; import { ServerConfig } from "../config.ts"; import * as VcsDriverRegistry from "../vcs/VcsDriverRegistry.ts"; import * as VcsProcess from "../vcs/VcsProcess.ts"; @@ -128,11 +132,12 @@ function selectProviderContext( remotes: ReadonlyArray<{ readonly name: string; readonly url: string; + readonly detectionUrl: string; }>, ): SourceControlProvider.SourceControlProviderContext | null { const candidates: Array = []; for (const remote of remotes) { - const provider = detectSourceControlProviderFromRemoteUrl(remote.url); + const provider = detectSourceControlProviderFromRemoteUrl(remote.detectionUrl); if (provider) { candidates.push({ provider, @@ -199,6 +204,9 @@ export const makeWithProviders = Effect.fn("makeSourceControlProviderRegistryWit const config = yield* ServerConfig; const process = yield* VcsProcess.VcsProcess; const vcsRegistry = yield* VcsDriverRegistry.VcsDriverRegistry; + const sshContext = yield* Effect.context(); + const resolveDetectionUrl = (remoteUrl: string) => + resolveGitRemoteForSourceControl(remoteUrl).pipe(Effect.provideContext(sshContext)); const providers = new Map< SourceControlProviderKind, SourceControlProvider.SourceControlProvider["Service"] @@ -234,7 +242,19 @@ export const makeWithProviders = Effect.fn("makeSourceControlProviderRegistryWit }), ), ); - const context = selectProviderContext(remotes.remotes); + const remotesForDetection = yield* Effect.forEach( + remotes.remotes, + (remote) => + resolveDetectionUrl(remote.url).pipe( + Effect.map((detectionUrl) => ({ + name: remote.name, + url: remote.url, + detectionUrl, + })), + ), + { concurrency: "unbounded" }, + ); + const context = selectProviderContext(remotesForDetection); return yield* refineUnknownRemoteProvider({ specs: discoverySpecs, diff --git a/apps/server/src/sourceControl/resolveGitRemoteForSourceControl.test.ts b/apps/server/src/sourceControl/resolveGitRemoteForSourceControl.test.ts new file mode 100644 index 000000000000..43336f0ad720 --- /dev/null +++ b/apps/server/src/sourceControl/resolveGitRemoteForSourceControl.test.ts @@ -0,0 +1,69 @@ +import { assert, describe, it } from "@effect/vitest"; +import { detectSourceControlProviderFromRemoteUrl } from "@t3tools/shared/sourceControl"; +import * as Effect from "effect/Effect"; + +import { + gitRemoteSshAliasToResolve, + resolveGitRemoteForSourceControl, +} from "./resolveGitRemoteForSourceControl.ts"; + +describe("gitRemoteSshAliasToResolve", () => { + it("selects undotted SSH aliases and skips real hostnames", () => { + assert.strictEqual( + gitRemoteSshAliasToResolve("git@github-personal:owner/repo.git"), + "github-personal", + ); + assert.strictEqual( + gitRemoteSshAliasToResolve("ssh://git@gitlab-work/group/project.git"), + "gitlab-work", + ); + assert.strictEqual(gitRemoteSshAliasToResolve("git@github.com:owner/repo.git"), null); + assert.strictEqual(gitRemoteSshAliasToResolve("https://github.com/owner/repo.git"), null); + }); +}); + +describe("resolveGitRemoteForSourceControl", () => { + it.effect("rewrites an SSH alias to the resolved HostName", () => + Effect.gen(function* () { + const rewritten = yield* resolveGitRemoteForSourceControl( + "git@github-personal:owner/repo.git", + () => Effect.succeed("github.com"), + ); + + assert.strictEqual(rewritten, "git@github.com:owner/repo.git"); + assert.deepStrictEqual(detectSourceControlProviderFromRemoteUrl(rewritten), { + kind: "github", + name: "GitHub", + baseUrl: "https://github.com", + }); + }), + ); + + it.effect("leaves the original remote when resolve fails", () => + Effect.gen(function* () { + const original = "git@github-personal:owner/repo.git"; + const rewritten = yield* resolveGitRemoteForSourceControl(original, () => + Effect.succeed(null), + ); + + assert.strictEqual(rewritten, original); + assert.strictEqual(detectSourceControlProviderFromRemoteUrl(rewritten)?.kind, "unknown"); + }), + ); + + it.effect("does not invent github.com and does not resolve dotted hosts", () => + Effect.gen(function* () { + let resolveCalls = 0; + const resolve = () => { + resolveCalls += 1; + return Effect.succeed("evil.example"); + }; + + assert.strictEqual( + yield* resolveGitRemoteForSourceControl("git@github.com:owner/repo.git", resolve), + "git@github.com:owner/repo.git", + ); + assert.strictEqual(resolveCalls, 0); + }), + ); +}); diff --git a/apps/server/src/sourceControl/resolveGitRemoteForSourceControl.ts b/apps/server/src/sourceControl/resolveGitRemoteForSourceControl.ts new file mode 100644 index 000000000000..2608f1910e8b --- /dev/null +++ b/apps/server/src/sourceControl/resolveGitRemoteForSourceControl.ts @@ -0,0 +1,100 @@ +import { isSshRemoteUrl, rewriteGitRemoteUrlHost } from "@t3tools/shared/sourceControl"; +import { resolveSshTarget } from "@t3tools/ssh/command"; +import * as Effect from "effect/Effect"; +import * as FileSystem from "effect/FileSystem"; +import * as Path from "effect/Path"; +import { ChildProcessSpawner } from "effect/unstable/process"; + +const SCP_SSH_HOST_PATTERN = /^[a-zA-Z0-9._-]+@([^:/]+):/; + +const KNOWN_PUBLIC_SSH_HOSTS = new Set([ + "github.com", + "gitlab.com", + "bitbucket.org", + "dev.azure.com", +]); + +export type ResolveGitRemoteServices = + | ChildProcessSpawner.ChildProcessSpawner + | FileSystem.FileSystem + | Path.Path; + +export type ResolveSshHostname = ( + alias: string, +) => Effect.Effect; + +function parseSshRemoteHostname(remoteUrl: string): string | null { + const trimmed = remoteUrl.trim(); + if (trimmed.length === 0 || !isSshRemoteUrl(trimmed)) { + return null; + } + + const scpMatch = SCP_SSH_HOST_PATTERN.exec(trimmed); + if (scpMatch?.[1]) { + return scpMatch[1].toLowerCase(); + } + + if (trimmed.toLowerCase().startsWith("ssh://")) { + try { + const hostname = new URL(trimmed).hostname.toLowerCase(); + return hostname.length > 0 ? hostname : null; + } catch { + return null; + } + } + + return null; +} + +/** + * SSH aliases such as `github-personal` have no dot and are not public hosts. + * Those are the only remotes worth asking `ssh -G` about. + */ +export function gitRemoteSshAliasToResolve(remoteUrl: string): string | null { + const hostname = parseSshRemoteHostname(remoteUrl); + if (hostname === null || hostname.includes(".") || KNOWN_PUBLIC_SSH_HOSTS.has(hostname)) { + return null; + } + return hostname; +} + +export const resolveSshHostnameFromConfig: ResolveSshHostname = (alias) => + resolveSshTarget(alias).pipe( + Effect.map((target) => { + const hostname = target.hostname.trim(); + return hostname.length > 0 ? hostname : null; + }), + Effect.orElseSucceed(() => null), + ); + +/** + * Rewrites `git@alias:path` to the HostName from `ssh -G` so provider + * detection and the PR pane talk to the real API host. The original remote + * is left unchanged when resolve fails or the host is already a real name. + */ +export function resolveGitRemoteForSourceControl( + remoteUrl: string, + resolveHostname: ResolveSshHostname, +): Effect.Effect; +export function resolveGitRemoteForSourceControl( + remoteUrl: string, +): Effect.Effect; +export function resolveGitRemoteForSourceControl( + remoteUrl: string, + resolveHostname: ResolveSshHostname = resolveSshHostnameFromConfig, +): Effect.Effect { + return Effect.gen(function* () { + const alias = gitRemoteSshAliasToResolve(remoteUrl); + if (alias === null) { + return remoteUrl; + } + + const resolved = yield* resolveHostname(alias); + const hostname = resolved?.trim() ?? ""; + if (hostname.length === 0) { + return remoteUrl; + } + + return rewriteGitRemoteUrlHost(remoteUrl, hostname); + }); +} diff --git a/docs/user/source-control.md b/docs/user/source-control.md index c64a63f7bc49..d72dd9ff9800 100644 --- a/docs/user/source-control.md +++ b/docs/user/source-control.md @@ -77,6 +77,8 @@ Run a quick **Rescan** after setting up a new machine or changing credentials. ``` 3. Open **Settings → Source Control** in T3 Code and verify GitHub shows as authenticated +If a repository uses an SSH host alias such as `github-personal` that points at `github.com`, T3 Code uses the real hostname for pull requests and keeps the alias for Git authentication. + You can now clone, publish, and create pull requests. ### For GitLab diff --git a/packages/shared/src/sourceControl.test.ts b/packages/shared/src/sourceControl.test.ts index 86b1ba5912bd..10d2212fc468 100644 --- a/packages/shared/src/sourceControl.test.ts +++ b/packages/shared/src/sourceControl.test.ts @@ -5,6 +5,7 @@ import { getChangeRequestTerminologyForKind, isSshRemoteUrl, resolveChangeRequestPresentation, + rewriteGitRemoteUrlHost, } from "./sourceControl.ts"; describe("source control presentation", () => { @@ -137,6 +138,61 @@ describe("detectSourceControlProviderFromRemoteUrl", () => { detectSourceControlProviderFromRemoteUrl("git@bitbucket.org:workspace/repo.git")?.kind, ).toBe("bitbucket"); }); + + it("does not treat undotted SSH aliases as enterprise hosts", () => { + expect(detectSourceControlProviderFromRemoteUrl("git@github-personal:owner/repo.git")).toEqual({ + kind: "unknown", + name: "github-personal", + baseUrl: "https://github-personal", + }); + expect(detectSourceControlProviderFromRemoteUrl("git@github:owner/repo.git")?.kind).toBe( + "unknown", + ); + expect( + detectSourceControlProviderFromRemoteUrl("git@gitlab-work:group/project.git")?.kind, + ).toBe("unknown"); + expect( + detectSourceControlProviderFromRemoteUrl("git@bitbucket-work:workspace/repo.git")?.kind, + ).toBe("unknown"); + }); + + it("still classifies public and dotted enterprise hosts", () => { + expect(detectSourceControlProviderFromRemoteUrl("git@github.com:owner/repo.git")).toEqual({ + kind: "github", + name: "GitHub", + baseUrl: "https://github.com", + }); + expect( + detectSourceControlProviderFromRemoteUrl("https://github.mycorp.com/owner/repo.git"), + ).toEqual({ + kind: "github", + name: "GitHub Self-Hosted", + baseUrl: "https://github.mycorp.com", + }); + expect(detectSourceControlProviderFromRemoteUrl("git@gitlab.com:group/project.git")?.kind).toBe( + "gitlab", + ); + expect( + detectSourceControlProviderFromRemoteUrl("git@bitbucket.org:workspace/repo.git")?.kind, + ).toBe("bitbucket"); + }); +}); + +describe("rewriteGitRemoteUrlHost", () => { + it("rewrites SCP-like SSH aliases and leaves HTTPS remotes alone", () => { + expect(rewriteGitRemoteUrlHost("git@github-personal:o/r.git", "github.com")).toBe( + "git@github.com:o/r.git", + ); + expect(rewriteGitRemoteUrlHost("deploy@github-personal:Owner/Repo.git", "github.com")).toBe( + "deploy@github.com:Owner/Repo.git", + ); + expect(rewriteGitRemoteUrlHost("ssh://git@github-personal/o/r.git", "github.com")).toBe( + "ssh://git@github.com/o/r.git", + ); + expect(rewriteGitRemoteUrlHost("https://github.com/o/r.git", "example.com")).toBe( + "https://github.com/o/r.git", + ); + }); }); describe("isSshRemoteUrl", () => { diff --git a/packages/shared/src/sourceControl.ts b/packages/shared/src/sourceControl.ts index df88de595a3f..c20f2f845b2f 100644 --- a/packages/shared/src/sourceControl.ts +++ b/packages/shared/src/sourceControl.ts @@ -133,13 +133,43 @@ export function getChangeRequestTerminologyForKind( }; } -const SCP_SSH_REMOTE_PATTERN = /^[a-zA-Z0-9._-]+@([^:/]+):/; +const SCP_SSH_REMOTE_PATTERN = /^([a-zA-Z0-9._-]+)@([^:/]+):(.*)$/; export function isSshRemoteUrl(remoteUrl: string): boolean { const trimmed = remoteUrl.trim(); return SCP_SSH_REMOTE_PATTERN.test(trimmed) || trimmed.toLowerCase().startsWith("ssh://"); } +/** + * Rewrites the host of an SCP-like or `ssh://` remote. HTTPS remotes are left + * unchanged so API/browser host mapping can reuse the git URL without touching + * clone auth. + */ +export function rewriteGitRemoteUrlHost(remoteUrl: string, canonicalHostname: string): string { + const trimmed = remoteUrl.trim(); + const hostname = canonicalHostname.trim().toLowerCase(); + if (trimmed.length === 0 || hostname.length === 0) { + return remoteUrl; + } + + const scpMatch = SCP_SSH_REMOTE_PATTERN.exec(trimmed); + if (scpMatch?.[1] !== undefined && scpMatch[2] !== undefined && scpMatch[3] !== undefined) { + return `${scpMatch[1]}@${hostname}:${scpMatch[3]}`; + } + + if (trimmed.toLowerCase().startsWith("ssh://")) { + try { + const url = new URL(trimmed); + url.hostname = hostname; + return url.toString(); + } catch { + return remoteUrl; + } + } + + return remoteUrl; +} + function parseRemoteHost(remoteUrl: string): string | null { const trimmed = remoteUrl.trim(); if (trimmed.length === 0) { @@ -147,8 +177,8 @@ function parseRemoteHost(remoteUrl: string): string | null { } const scpMatch = SCP_SSH_REMOTE_PATTERN.exec(trimmed); - if (scpMatch?.[1]) { - return scpMatch[1].toLowerCase(); + if (scpMatch?.[2]) { + return scpMatch[2].toLowerCase(); } try { @@ -174,12 +204,19 @@ function hasDnsLabel(host: string, label: string): boolean { return host.split(".").includes(label); } +function isPublicOrDottedLabelHost(host: string, publicHost: string, label: string): boolean { + // Undotted SSH aliases (`github-personal`, `gitlab-work`) share a substring + // with the public host but are not enterprise installs. Require a real + // hostname (a dot) before treating a label match as self-hosted. + return host === publicHost || (host.includes(".") && hasDnsLabel(host, label)); +} + function isGitHubHost(host: string): boolean { - return host === "github.com" || hasDnsLabel(host, "github"); + return isPublicOrDottedLabelHost(host, "github.com", "github"); } function isGitLabHost(host: string): boolean { - return host === "gitlab.com" || hasDnsLabel(host, "gitlab"); + return isPublicOrDottedLabelHost(host, "gitlab.com", "gitlab"); } function isAzureDevOpsHost(host: string): boolean { @@ -195,7 +232,7 @@ function isAzureDevOpsHost(host: string): boolean { } function isBitbucketHost(host: string): boolean { - return host === "bitbucket.org" || hasDnsLabel(host, "bitbucket"); + return isPublicOrDottedLabelHost(host, "bitbucket.org", "bitbucket"); } export function detectSourceControlProviderFromRemoteUrl( diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 2c79aea36a0e..f31c4778b650 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -492,6 +492,9 @@ importers: '@t3tools/shared': specifier: workspace:* version: link:../../packages/shared + '@t3tools/ssh': + specifier: workspace:* + version: link:../../packages/ssh '@t3tools/tailscale': specifier: workspace:* version: link:../../packages/tailscale From 9c2eaa931129284a798f3fdb170b9eb88604531a Mon Sep 17 00:00:00 2001 From: maslin Date: Sun, 16 Aug 2026 16:37:42 +0700 Subject: [PATCH 2/2] fix(vcs): keep self-hosted ports and bound SSH alias probes --- .../pullRequest/PullRequestService.test.ts | 50 ++++++++++ .../src/pullRequest/PullRequestService.ts | 10 +- .../resolveGitRemoteForSourceControl.test.ts | 93 +++++++++++++++++++ .../resolveGitRemoteForSourceControl.ts | 8 +- packages/ssh/src/command.ts | 15 ++- 5 files changed, 170 insertions(+), 6 deletions(-) diff --git a/apps/server/src/pullRequest/PullRequestService.test.ts b/apps/server/src/pullRequest/PullRequestService.test.ts index 8046f85c44e9..16d5be6f2d5e 100644 --- a/apps/server/src/pullRequest/PullRequestService.test.ts +++ b/apps/server/src/pullRequest/PullRequestService.test.ts @@ -176,6 +176,56 @@ function makeService(input: { ); } +it("keeps a nonstandard port when deriving a host from a provider base URL", () => { + assert.strictEqual( + PullRequestService.hostnameFromProviderBaseUrl("https://github.company:8443"), + "github.company:8443", + ); + assert.strictEqual( + PullRequestService.hostnameFromProviderBaseUrl("https://github.company:8443/api/v3"), + "github.company:8443", + ); + assert.strictEqual( + PullRequestService.hostnameFromProviderBaseUrl("https://github.com"), + "github.com", + ); +}); + +it.effect("keeps a nonstandard port on a detected self-hosted GitHub host", () => + Effect.gen(function* () { + const hosts: string[] = []; + const selfHosted = project({ + id: "p1", + title: "ghe", + workspaceRoot: "/ghe", + repository: "owner/repo", + provider: "unknown", + host: "github.company:8443", + }); + const service = yield* makeService({ + projects: [selfHosted], + providers: [ + fakeProvider("github", { + listChangeRequests: (input) => { + hosts.push(input.host); + return Effect.succeed({ items: [], truncated: false, continues: true }); + }, + }), + ], + resolveHandle: ({ context }) => + Effect.succeed({ + context: { ...context!, provider: { ...context!.provider, kind: "github" } }, + provider: undefined as never, + }), + }); + + const result = yield* service.list({ state: "open" }); + + assert.strictEqual(result.providers[0]?.host, "github.company:8443"); + assert.deepStrictEqual(hosts, ["github.company:8443"]); + }), +); + it.effect("refines unknown self-hosted GitLab projects before listing merge requests", () => Effect.gen(function* () { let refinementCalls = 0; diff --git a/apps/server/src/pullRequest/PullRequestService.ts b/apps/server/src/pullRequest/PullRequestService.ts index b0e1ae78d17e..0da552a58dea 100644 --- a/apps/server/src/pullRequest/PullRequestService.ts +++ b/apps/server/src/pullRequest/PullRequestService.ts @@ -394,10 +394,14 @@ export function repositoryIdentityOf(project: OrchestrationProjectShell): string return identity.owner && identity.name ? `${identity.owner}/${identity.name}` : null; } -function hostnameFromProviderBaseUrl(baseUrl: string): string | null { +/** + * Host a detected provider base URL should be stored as. `URL.hostname` drops a + * nonstandard port, so `https://github.company:8443` must use `URL.host`. + */ +export function hostnameFromProviderBaseUrl(baseUrl: string): string | null { try { - const hostname = new URL(baseUrl).hostname.toLowerCase(); - return hostname.length > 0 ? hostname : null; + const host = new URL(baseUrl).host.toLowerCase(); + return host.length > 0 ? host : null; } catch { return null; } diff --git a/apps/server/src/sourceControl/resolveGitRemoteForSourceControl.test.ts b/apps/server/src/sourceControl/resolveGitRemoteForSourceControl.test.ts index 43336f0ad720..76e6f2587d54 100644 --- a/apps/server/src/sourceControl/resolveGitRemoteForSourceControl.test.ts +++ b/apps/server/src/sourceControl/resolveGitRemoteForSourceControl.test.ts @@ -1,12 +1,66 @@ import { assert, describe, it } from "@effect/vitest"; +import * as NodeServices from "@effect/platform-node/NodeServices"; import { detectSourceControlProviderFromRemoteUrl } from "@t3tools/shared/sourceControl"; +import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; +import * as Fiber from "effect/Fiber"; +import * as Layer from "effect/Layer"; +import * as Sink from "effect/Sink"; +import * as Stream from "effect/Stream"; +import * as TestClock from "effect/testing/TestClock"; +import { ChildProcessSpawner } from "effect/unstable/process"; import { gitRemoteSshAliasToResolve, resolveGitRemoteForSourceControl, + SSH_CONFIG_RESOLVE_TIMEOUT_MS, } from "./resolveGitRemoteForSourceControl.ts"; +const encoder = new TextEncoder(); + +const makeFailedProcess = (input: { readonly stdout: string; readonly stderr?: string }) => { + const stdoutStream = Stream.make(encoder.encode(input.stdout)); + const stderrStream = input.stderr ? Stream.make(encoder.encode(input.stderr)) : Stream.empty; + return ChildProcessSpawner.makeHandle({ + pid: ChildProcessSpawner.ProcessId(123), + stdout: stdoutStream, + stderr: stderrStream, + all: Stream.empty, + exitCode: Effect.succeed(ChildProcessSpawner.ExitCode(1)), + isRunning: Effect.succeed(false), + kill: () => Effect.void, + stdin: Sink.drain, + getInputFd: () => Sink.drain, + getOutputFd: () => Stream.empty, + unref: Effect.succeed(Effect.void), + }); +}; + +const makeNeverFinishingProcess = () => { + let finish: ((exitCode: ChildProcessSpawner.ExitCode) => void) | null = null; + return ChildProcessSpawner.makeHandle({ + pid: ChildProcessSpawner.ProcessId(123), + stdout: Stream.empty, + stderr: Stream.empty, + all: Stream.empty, + exitCode: Effect.callback((resume) => { + finish = (exitCode) => resume(Effect.succeed(exitCode)); + return Effect.sync(() => { + finish = null; + }); + }), + isRunning: Effect.succeed(true), + kill: () => + Effect.sync(() => { + finish?.(ChildProcessSpawner.ExitCode(143)); + }), + stdin: Sink.drain, + getInputFd: () => Sink.drain, + getOutputFd: () => Stream.empty, + unref: Effect.succeed(Effect.void), + }); +}; + describe("gitRemoteSshAliasToResolve", () => { it("selects undotted SSH aliases and skips real hostnames", () => { assert.strictEqual( @@ -66,4 +120,43 @@ describe("resolveGitRemoteForSourceControl", () => { assert.strictEqual(resolveCalls, 0); }), ); + + it.effect("leaves the original remote when ssh -G fails", () => { + const spawner = ChildProcessSpawner.make(() => + Effect.succeed(makeFailedProcess({ stdout: "", stderr: "ssh: Could not resolve hostname" })), + ); + const processLayer = Layer.mergeAll( + NodeServices.layer, + Layer.succeed(ChildProcessSpawner.ChildProcessSpawner, spawner), + ); + + return Effect.gen(function* () { + const original = "git@github-personal:owner/repo.git"; + const rewritten = yield* resolveGitRemoteForSourceControl(original); + + assert.strictEqual(rewritten, original); + assert.strictEqual(detectSourceControlProviderFromRemoteUrl(rewritten)?.kind, "unknown"); + }).pipe(Effect.provide(processLayer)); + }); + + it.effect("leaves the original remote when ssh -G times out", () => { + const spawner = ChildProcessSpawner.make(() => Effect.succeed(makeNeverFinishingProcess())); + const processLayer = Layer.mergeAll( + NodeServices.layer, + Layer.succeed(ChildProcessSpawner.ChildProcessSpawner, spawner), + TestClock.layer(), + ); + + return Effect.gen(function* () { + const original = "git@github-personal:owner/repo.git"; + const fiber = yield* Effect.forkChild(resolveGitRemoteForSourceControl(original)); + yield* Effect.yieldNow; + yield* TestClock.adjust(Duration.millis(SSH_CONFIG_RESOLVE_TIMEOUT_MS)); + + const rewritten = yield* Fiber.join(fiber); + + assert.strictEqual(rewritten, original); + assert.strictEqual(detectSourceControlProviderFromRemoteUrl(rewritten)?.kind, "unknown"); + }).pipe(Effect.provide(processLayer)); + }); }); diff --git a/apps/server/src/sourceControl/resolveGitRemoteForSourceControl.ts b/apps/server/src/sourceControl/resolveGitRemoteForSourceControl.ts index 2608f1910e8b..2991cca4f4ea 100644 --- a/apps/server/src/sourceControl/resolveGitRemoteForSourceControl.ts +++ b/apps/server/src/sourceControl/resolveGitRemoteForSourceControl.ts @@ -58,8 +58,14 @@ export function gitRemoteSshAliasToResolve(remoteUrl: string): string | null { return hostname; } +/** Same budget as `RepositoryIdentityResolver`'s `ssh -G` probe. */ +export const SSH_CONFIG_RESOLVE_TIMEOUT_MS = 5_000; + export const resolveSshHostnameFromConfig: ResolveSshHostname = (alias) => - resolveSshTarget(alias).pipe( + resolveSshTarget(alias, { + timeoutMs: SSH_CONFIG_RESOLVE_TIMEOUT_MS, + fallbackOnError: false, + }).pipe( Effect.map((target) => { const hostname = target.hostname.trim(); return hostname.length > 0 ? hostname : null; diff --git a/packages/ssh/src/command.ts b/packages/ssh/src/command.ts index 10927b43089c..42d39b988027 100644 --- a/packages/ssh/src/command.ts +++ b/packages/ssh/src/command.ts @@ -327,6 +327,7 @@ export const runSshCommand = Effect.fn("ssh/command.runSshCommand")(function* ( export const resolveSshTarget = Effect.fn("ssh/command.resolveSshTarget")(function* ( alias: string, + options?: { readonly timeoutMs?: number; readonly fallbackOnError?: boolean }, ): Effect.fn.Return< DesktopSshEnvironmentTarget, SshCommandError | SshInvalidTargetError, @@ -338,19 +339,29 @@ export const resolveSshTarget = Effect.fn("ssh/command.resolveSshTarget")(functi } yield* Effect.logDebug("ssh.target.resolve.start", { alias: trimmedAlias }); - return yield* runSshCommand( + const resolved = runSshCommand( { alias: trimmedAlias, hostname: trimmedAlias, username: null, port: null, }, - { preHostArgs: ["-G"] }, + { + preHostArgs: ["-G"], + ...(options?.timeoutMs === undefined ? {} : { timeoutMs: options.timeoutMs }), + }, ).pipe( Effect.map((result) => parseSshResolveOutput(trimmedAlias, result.stdout)), Effect.tap((target) => Effect.logDebug("ssh.target.resolve.succeeded", sshTargetLogFields(target)), ), + ); + + if (options?.fallbackOnError === false) { + return yield* resolved; + } + + return yield* resolved.pipe( Effect.catch((cause) => Effect.logDebug("ssh.target.resolve.fallback", { alias: trimmedAlias, cause }).pipe( Effect.as({