fix: Handle SSH over HTTPS GitHub remote URLs

apps/code/src/main/services/folders/service.test.ts

@@ -93,7 +93,7 @@ vi.mock("../../db/repositories/worktree-repository.js", () => ({
   WorktreeRepository: vi.fn(() => mockWorktreeRepo),
 }));
 
-import { isGitRepository } from "@posthog/git/queries";
+import { getRemoteUrl, isGitRepository } from "@posthog/git/queries";
 import type { IDialog } from "@posthog/platform/dialog";
 import type { IRepositoryRepository } from "../../db/repositories/repository-repository";
 import type { IWorkspaceRepository } from "../../db/repositories/workspace-repository";
@@ -467,6 +467,32 @@ describe("FoldersService", () => {
       });
     });
 
+    it("normalizes a non-GitHub override and skips the local remote lookup", async () => {
+      vi.mocked(isGitRepository).mockResolvedValue(true);
+      vi.mocked(getRemoteUrl).mockResolvedValue(
+        "https://github.com/SomeoneElse/wrong",
+      );
+      mockRepositoryRepo.findByPath.mockReturnValue(null);
+      mockRepositoryRepo.create.mockReturnValue({
+        id: "folder-new",
+        path: "/home/user/fork",
+        remoteUrl: "https://gitlab.com/PostHog/posthog",
+        lastAccessedAt: "2024-01-01T00:00:00.000Z",
+        createdAt: "2024-01-01T00:00:00.000Z",
+        updatedAt: "2024-01-01T00:00:00.000Z",
+      });
+
+      await service.addFolder("/home/user/fork", {
+        remoteUrl: "https://gitlab.com/PostHog/posthog.git",
+      });
+
+      expect(mockRepositoryRepo.create).toHaveBeenCalledWith({
+        path: "/home/user/fork",
+        remoteUrl: "https://gitlab.com/PostHog/posthog",
+      });
+      expect(getRemoteUrl).not.toHaveBeenCalled();
+    });
+
     it("backfills remoteUrl on an existing folder when override is supplied", async () => {
       vi.mocked(isGitRepository).mockResolvedValue(true);
       const existing = {

apps/code/src/main/services/folders/service.ts

@@ -2,21 +2,10 @@ import fs from "node:fs";
 import path from "node:path";
 import { getRemoteUrl, isGitRepository } from "@posthog/git/queries";
 import { InitRepositorySaga } from "@posthog/git/sagas/init";
-
-import { normalizeRepoKey } from "@shared/utils/repo";
-
-function extractRepoKey(url: string): string | null {
-  const httpsMatch = url.match(/github\.com\/([^/]+\/[^/]+)/);
-  if (httpsMatch) return normalizeRepoKey(httpsMatch[1]);
-
-  const sshMatch = url.match(/github\.com:([^/]+\/[^/]+)/);
-  if (sshMatch) return normalizeRepoKey(sshMatch[1]);
-
-  return null;
-}
-
+import { parseGitHubUrl } from "@posthog/git/utils";
 import { WorktreeManager } from "@posthog/git/worktree";
 import type { IDialog } from "@posthog/platform/dialog";
+import { normalizeRepoKey } from "@shared/utils/repo";
 import { inject, injectable } from "inversify";
 import type {
   IRepositoryRepository,
@@ -249,12 +238,13 @@ export class FoldersService {
     overrideRemoteUrl: string | undefined,
   ): Promise<string | null> {
     if (overrideRemoteUrl) {
-      const overrideKey = extractRepoKey(overrideRemoteUrl);
-      if (overrideKey) return overrideKey;
-      return normalizeRepoKey(overrideRemoteUrl);
+      return (
+        parseGitHubUrl(overrideRemoteUrl)?.path ??
+        normalizeRepoKey(overrideRemoteUrl)
+      );
     }
     const localRemoteUrl = await getRemoteUrl(folderPath);
-    return localRemoteUrl ? extractRepoKey(localRemoteUrl) : null;
+    return parseGitHubUrl(localRemoteUrl)?.path ?? null;
   }
 
   getRepositoryByRemoteUrl(

apps/code/src/main/services/git/service.ts

@@ -434,7 +434,7 @@ export class GitService extends TypedEventEmitter<GitServiceEvents> {
 
       let compareUrl: string | null = null;
       if (currentBranch && currentBranch !== defaultBranch) {
-        compareUrl = `https://github.com/${parsed.organization}/${parsed.repository}/compare/${defaultBranch}...${currentBranch}?expand=1`;
+        compareUrl = `https://github.com/${parsed.path}/compare/${defaultBranch}...${currentBranch}?expand=1`;
       }
 
       return {
@@ -893,7 +893,6 @@ export class GitService extends TypedEventEmitter<GitServiceEvents> {
       const parsed = parseGitHubUrl(remoteUrl);
       if (!parsed) return null;
 
-      const repoSlug = `${parsed.organization}/${parsed.repository}`;
       const result = await execGh([
         "pr",
         "list",
@@ -906,7 +905,7 @@ export class GitService extends TypedEventEmitter<GitServiceEvents> {
         "--limit",
         "1",
         "--repo",
-        repoSlug,
+        parsed.path,
       ]);
 
       if (result.exitCode !== 0) {

packages/git/package.json

@@ -18,15 +18,16 @@
   },
   "devDependencies": {
     "@types/tar": "^6.1.13",
-    "vitest": "^2.1.8",
-    "typescript": "^5.5.0"
+    "typescript": "^5.5.0",
+    "vitest": "^2.1.8"
   },
   "files": [
     "dist/**/*",
     "src/**/*"
   ],
   "dependencies": {
     "@posthog/shared": "workspace:*",
+    "git-url-parse": "^16.1.0",
     "simple-git": "^3.30.0"
   }
 }

packages/git/src/utils.test.ts

@@ -2,7 +2,7 @@ import { chmod, mkdir, mkdtemp, rm, stat, writeFile } from "node:fs/promises";
 import { tmpdir } from "node:os";
 import path from "node:path";
 import { afterEach, describe, expect, it } from "vitest";
-import { forceRemove } from "./utils";
+import { forceRemove, parseGitHubUrl, parsePrUrl } from "./utils";
 
 async function fileExists(p: string): Promise<boolean> {
   try {
@@ -66,3 +66,182 @@ describe("forceRemove", () => {
     expect(await fileExists(target)).toBe(false);
   });
 });
+
+describe("parseGitHubUrl", () => {
+  it.each([
+    // HTTPS canonical forms
+    ["https://github.com/PostHog/code.git", "PostHog", "code"],
+    ["https://github.com/PostHog/code", "PostHog", "code"],
+    ["https://github.com/PostHog/code/", "PostHog", "code"],
+    ["https://github.com/PostHog/code.git/", "PostHog", "code"],
+    ["http://github.com/PostHog/code.git", "PostHog", "code"],
+    ["https://user:token@github.com/PostHog/code.git", "PostHog", "code"],
+    // SCP-style SSH
+    ["git@github.com:PostHog/code.git", "PostHog", "code"],
+    ["git@github.com:PostHog/code", "PostHog", "code"],
+    // ssh:// SSH variants
+    ["ssh://git@github.com/PostHog/code.git", "PostHog", "code"],
+    ["ssh://github.com/PostHog/code.git", "PostHog", "code"],
+    ["ssh://git@ssh.github.com:443/PostHog/code.git", "PostHog", "code"],
+    [
+      "ssh://git@ssh.github.com:443/buildingapplications/bilt-landing.git",
+      "buildingapplications",
+      "bilt-landing",
+    ],
+    ["ssh://git@github.com:22/PostHog/code.git", "PostHog", "code"],
+    // Other protocols
+    ["git://github.com/PostHog/code.git", "PostHog", "code"],
+    ["git+https://github.com/PostHog/code.git", "PostHog", "code"],
+    ["git+ssh://git@github.com/PostHog/code.git", "PostHog", "code"],
+    // Whitespace + shorthand
+    ["  https://github.com/PostHog/code.git\n", "PostHog", "code"],
+    ["\thttps://github.com/PostHog/code.git", "PostHog", "code"],
+    ["PostHog/code", "PostHog", "code"],
+    // Web URLs (path markers git-url-parse recognises)
+    ["https://github.com/PostHog/code/blob/main/README.md", "PostHog", "code"],
+    ["https://github.com/PostHog/code/tree/main", "PostHog", "code"],
+    ["https://github.com/PostHog/code/issues/12", "PostHog", "code"],
+    ["https://github.com/PostHog/code/commit/abc123", "PostHog", "code"],
+    // Mixed-case host (case in path is preserved)
+    ["git@GitHub.com:PostHog/Code.git", "PostHog", "Code"],
+    ["https://GITHUB.COM/PostHog/code.git", "PostHog", "code"],
+    ["HTTPS://github.com/PostHog/code.git", "PostHog", "code"],
+    // Query strings + fragments
+    ["https://github.com/PostHog/code.git?ref=main", "PostHog", "code"],
+    ["https://github.com/PostHog/code#readme", "PostHog", "code"],
+    // Special characters
+    ["https://github.com/post-hog/my-cool-repo", "post-hog", "my-cool-repo"],
+    ["https://github.com/PostHog/dotted.repo", "PostHog", "dotted.repo"],
+    ["https://github.com/Post_Hog/repo_name", "Post_Hog", "repo_name"],
+    ["https://github.com/123/456", "123", "456"],
+  ])("parses %s", (url, organization, repository) => {
+    expect(parseGitHubUrl(url)).toEqual({
+      organization,
+      repository,
+      path: `${organization}/${repository}`,
+    });
+  });
+
+  it.each<string | null | undefined>([
+    // Empty / nullish
+    "",
+    "   ",
+    "\t\n",
+    null,
+    undefined,
+    // Non-URL strings
+    "not-a-url",
+    "PostHog",
+    "github.com/PostHog/code",
+    "//github.com/PostHog/code",
+    // Wrong host
+    "https://gitlab.com/PostHog/code.git",
+    "https://example.com/PostHog/code.git",
+    "git@gitlab.com:PostHog/code.git",
+    // SSH host alias (e.g. ~/.ssh/config Host github-personal). The remote may
+    // resolve to GitHub at connect time, but we can't know that statically.
+    "git@my-alias:PostHog/code.git",
+    "https://raw.githubusercontent.com/PostHog/code/main/README.md",
+    "file:///path/to/repo",
+    // Missing repo
+    "https://github.com/PostHog",
+    // Multiple / leading slashes
+    "https://github.com//PostHog/code.git",
+    "https://github.com/PostHog//code.git",
+    // Subdomains we don't trust
+    "https://api.github.com/repos/PostHog/code",
+    // GitHub web tabs git-url-parse can't isolate the repo from
+    "https://github.com/PostHog/code/wiki",
+    "https://github.com/PostHog/code/actions",
+    "https://github.com/PostHog/code/releases/tag/v1.0.0",
+    "https://github.com/PostHog/code/pull/42",
+  ])("returns null for %s", (url) => {
+    expect(parseGitHubUrl(url)).toBeNull();
+  });
+});
+
+describe("parsePrUrl", () => {
+  it.each([
+    // Canonical PR URLs
+    ["https://github.com/PostHog/code/pull/42", "PostHog", "code", 42],
+    ["http://github.com/PostHog/code/pull/1", "PostHog", "code", 1],
+    [
+      "https://github.com/buildingapplications/bilt-landing/pull/123",
+      "buildingapplications",
+      "bilt-landing",
+      123,
+    ],
+    // Whitespace
+    ["  https://github.com/PostHog/code/pull/7\n", "PostHog", "code", 7],
+    // PR sub-pages and tabs
+    ["https://github.com/PostHog/code/pull/42/files", "PostHog", "code", 42],
+    ["https://github.com/PostHog/code/pull/42/commits", "PostHog", "code", 42],
+    [
+      "https://github.com/PostHog/code/pull/42/commits/abc123",
+      "PostHog",
+      "code",
+      42,
+    ],
+    ["https://github.com/PostHog/code/pull/42/checks", "PostHog", "code", 42],
+    // Query strings + fragments
+    [
+      "https://github.com/PostHog/code/pull/42?diff=split",
+      "PostHog",
+      "code",
+      42,
+    ],
+    [
+      "https://github.com/PostHog/code/pull/42#discussion_r123",
+      "PostHog",
+      "code",
+      42,
+    ],
+    [
+      "https://github.com/PostHog/code/pull/42/files#diff-abc",
+      "PostHog",
+      "code",
+      42,
+    ],
+    // Mixed-case host
+    ["https://GITHUB.COM/PostHog/code/pull/42", "PostHog", "code", 42],
+    // Special characters in owner/repo
+    ["https://github.com/post-hog/my-repo/pull/42", "post-hog", "my-repo", 42],
+    // Large numbers (still valid integers)
+    ["https://github.com/PostHog/code/pull/999999", "PostHog", "code", 999999],
+  ])("parses %s", (url, owner, repo, number) => {
+    expect(parsePrUrl(url)).toEqual({ owner, repo, number });
+  });
+
+  it.each<string | null | undefined>([
+    // Empty / nullish
+    "",
+    "   ",
+    null,
+    undefined,
+    // Not a URL
+    "not-a-url",
+    // Missing /pull
+    "https://github.com/PostHog/code",
+    "git@github.com:PostHog/code.git",
+    // Wrong path keyword
+    "https://github.com/PostHog/code/issues/42",
+    "https://github.com/PostHog/code/pulls/42",
+    "https://github.com/PostHog/code/discussions/42",
+    // Bad number
+    "https://github.com/PostHog/code/pull/abc",
+    "https://github.com/PostHog/code/pull/0",
+    "https://github.com/PostHog/code/pull/-1",
+    "https://github.com/PostHog/code/pull/42.5",
+    "https://github.com/PostHog/code/pull/",
+    "https://github.com/PostHog/code/pull",
+    // Double / leading slashes in the path
+    "https://github.com/PostHog/code//pull/42",
+    "https://github.com/PostHog/code/pull//42",
+    "https://github.com//PostHog/code/pull/42",
+    // Wrong host
+    "https://gitlab.com/PostHog/code/pull/42",
+    "https://api.github.com/repos/PostHog/code/pulls/42",
+  ])("returns null for %s", (url) => {
+    expect(parsePrUrl(url)).toBeNull();
+  });
+});

packages/git/src/utils.ts

@@ -2,10 +2,12 @@ import { execFile } from "node:child_process";
 import * as fs from "node:fs/promises";
 import * as os from "node:os";
 import * as path from "node:path";
+import gitUrlParse from "git-url-parse";
 
 export interface GitHubRepo {
   organization: string;
   repository: string;
+  path: string;
 }
 
 export async function safeSymlink(
@@ -158,21 +160,41 @@ export interface GitHubPr {
   number: number;
 }
 
-export function parsePrUrl(prUrl: string): GitHubPr | null {
-  const match = prUrl.match(/github\.com\/([^/]+)\/([^/]+)\/pull\/(\d+)/);
-  if (!match) return null;
-  return { owner: match[1], repo: match[2], number: Number(match[3]) };
+export function parsePrUrl(prUrl: string | null | undefined): GitHubPr | null {
+  if (!prUrl) return null;
+  let parsed: gitUrlParse.GitUrl;
+  try {
+    parsed = gitUrlParse(prUrl.trim());
+  } catch {
+    return null;
+  }
+  if (parsed.source.toLowerCase() !== "github.com" || !parsed.full_name) {
+    return null;
+  }
+  const [owner, repo, kind, num] = parsed.full_name.split("/");
+  if (!owner || !repo || kind !== "pull") return null;
+  const number = Number(num);
+  if (!Number.isInteger(number) || number <= 0) return null;
+  return { owner, repo, number };
 }
 
-export function parseGitHubUrl(url: string): GitHubRepo | null {
-  // Trim whitespace/newlines that git commands may include
-  const trimmedUrl = url.trim();
-
-  const match =
-    trimmedUrl.match(/github\.com[:/](.+?)\/(.+?)(\.git)?$/) ||
-    trimmedUrl.match(/git@github\.com:(.+?)\/(.+?)(\.git)?$/);
-
-  if (!match) return null;
-
-  return { organization: match[1], repository: match[2].replace(/\.git$/, "") };
+export function parseGitHubUrl(
+  url: string | null | undefined,
+): GitHubRepo | null {
+  if (!url) return null;
+  let parsed: gitUrlParse.GitUrl;
+  try {
+    parsed = gitUrlParse(url.trim());
+  } catch {
+    return null;
+  }
+  if (parsed.source.toLowerCase() !== "github.com") return null;
+  // git-url-parse stuffs unhandled path segments into owner (e.g. wiki, actions,
+  // releases pages), so reject anything that didn't cleanly split into org/repo.
+  if (!parsed.owner || !parsed.name || parsed.owner.includes("/")) return null;
+  return {
+    organization: parsed.owner,
+    repository: parsed.name,
+    path: `${parsed.owner}/${parsed.name}`,
+  };
 }