fix: improve git URL parsing

git/git.go

@@ -10,9 +10,9 @@ import (
 	"os"
 	"strings"
 
+	"github.com/coder/envbuilder/internal/ebutil"
 	"github.com/coder/envbuilder/options"
 
-	giturls "github.com/chainguard-dev/git-urls"
 	"github.com/go-git/go-billy/v5"
 	"github.com/go-git/go-git/v5"
 	"github.com/go-git/go-git/v5/plumbing"
@@ -49,18 +49,17 @@ type CloneRepoOptions struct {
 //
 // The bool returned states whether the repository was cloned or not.
 func CloneRepo(ctx context.Context, logf func(string, ...any), opts CloneRepoOptions) (bool, error) {
-	parsed, err := giturls.Parse(opts.RepoURL)
+	parsed, err := ebutil.ParseRepoURL(opts.RepoURL)
 	if err != nil {
 		return false, fmt.Errorf("parse url %q: %w", opts.RepoURL, err)
 	}
-	logf("Parsed Git URL as %q", parsed.Redacted())
 
 	thinPack := true
 
 	if !opts.ThinPack {
 		thinPack = false
 		logf("ThinPack options is false, Marking thin-pack as unsupported")
-	} else if parsed.Hostname() == "dev.azure.com" {
+	} else if parsed.Host == "dev.azure.com" {
 		// Azure DevOps requires capabilities multi_ack / multi_ack_detailed,
 		// which are not fully implemented and by default are included in
 		// transport.UnsupportedCapabilities.
@@ -92,12 +91,9 @@ func CloneRepo(ctx context.Context, logf func(string, ...any), opts CloneRepoOpt
 	if err != nil {
 		return false, fmt.Errorf("mkdir %q: %w", opts.Path, err)
 	}
-	reference := parsed.Fragment
-	if reference == "" && opts.SingleBranch {
-		reference = "refs/heads/main"
+	if parsed.Reference == "" && opts.SingleBranch {
+		parsed.Reference = "refs/heads/main"
 	}
-	parsed.RawFragment = ""
-	parsed.Fragment = ""
 	fs, err := opts.Storage.Chroot(opts.Path)
 	if err != nil {
 		return false, fmt.Errorf("chroot %q: %w", opts.Path, err)
@@ -120,10 +116,10 @@ func CloneRepo(ctx context.Context, logf func(string, ...any), opts CloneRepoOpt
 	}
 
 	_, err = git.CloneContext(ctx, gitStorage, fs, &git.CloneOptions{
-		URL:             parsed.String(),
+		URL:             opts.RepoURL, // Use the EXACT URL provided.
 		Auth:            opts.RepoAuth,
 		Progress:        opts.Progress,
-		ReferenceName:   plumbing.ReferenceName(reference),
+		ReferenceName:   plumbing.ReferenceName(parsed.Reference),
 		InsecureSkipTLS: opts.Insecure,
 		Depth:           opts.Depth,
 		SingleBranch:    opts.SingleBranch,
@@ -250,13 +246,13 @@ func SetupRepoAuth(logf func(string, ...any), options *options.Options) transpor
 		logf("❔ No Git URL supplied!")
 		return nil
 	}
-	parsedURL, err := giturls.Parse(options.GitURL)
+	parsedURL, err := ebutil.ParseRepoURL(options.GitURL)
 	if err != nil {
 		logf("❌ Failed to parse Git URL: %s", err.Error())
 		return nil
 	}
 
-	if parsedURL.Scheme == "http" || parsedURL.Scheme == "https" {
+	if parsedURL.Protocol == "http" || parsedURL.Protocol == "https" {
 		// Special case: no auth
 		if options.GitUsername == "" && options.GitPassword == "" {
 			logf("👤 Using no authentication!")
@@ -272,7 +268,7 @@ func SetupRepoAuth(logf func(string, ...any), options *options.Options) transpor
 		}
 	}
 
-	if parsedURL.Scheme == "file" {
+	if parsedURL.Protocol == "file" {
 		// go-git will try to fallback to using the `git` command for local
 		// filesystem clones. However, it's more likely than not that the
 		// `git` command is not present in the container image. Log a warning

git/git_test.go

@@ -21,6 +21,7 @@ import (
 	"github.com/go-git/go-billy/v5"
 	"github.com/go-git/go-billy/v5/memfs"
 	"github.com/go-git/go-billy/v5/osfs"
+	gittransport "github.com/go-git/go-git/v5/plumbing/transport"
 	githttp "github.com/go-git/go-git/v5/plumbing/transport/http"
 	gitssh "github.com/go-git/go-git/v5/plumbing/transport/ssh"
 	"github.com/stretchr/testify/require"
@@ -140,9 +141,9 @@ func TestCloneRepo(t *testing.T) {
 				authMW := mwtest.BasicAuthMW(tc.srvUsername, tc.srvPassword)
 				srv := httptest.NewServer(authMW(gittest.NewServer(srvFS)))
 
-				authURL, err := url.Parse(srv.URL)
+				authURL, err := gittransport.NewEndpoint(srv.URL)
 				require.NoError(t, err)
-				authURL.User = url.UserPassword(tc.username, tc.password)
+				authURL.User = url.UserPassword(tc.username, tc.password).String()
 				clientFS := memfs.New()
 
 				cloned, err := git.CloneRepo(context.Background(), t.Logf, git.CloneRepoOptions{
@@ -238,10 +239,9 @@ func TestShallowCloneRepo(t *testing.T) {
 func TestCloneRepoSSH(t *testing.T) {
 	t.Parallel()
 
-	t.Run("AuthSuccess", func(t *testing.T) {
+	t.Run("Success", func(t *testing.T) {
 		t.Parallel()
 
-		// TODO: test the rest of the cloning flow. This just tests successful auth.
 		tmpDir := t.TempDir()
 		srvFS := osfs.New(tmpDir, osfs.WithChrootOS())
 
@@ -264,10 +264,9 @@ func TestCloneRepoSSH(t *testing.T) {
 				},
 			},
 		})
-		// TODO: ideally, we want to test the entire cloning flow.
-		// For now, this indicates successful ssh key auth.
-		require.ErrorContains(t, err, "repository not found")
-		require.False(t, cloned)
+		require.NoError(t, err)
+		require.True(t, cloned)
+		require.Equal(t, "Hello, world!", mustRead(t, clientFS, "/workspace/README.md"))
 	})
 
 	t.Run("AuthFailure", func(t *testing.T) {
@@ -404,7 +403,7 @@ func TestSetupRepoAuth(t *testing.T) {
 		}
 		auth := git.SetupRepoAuth(t.Logf, opts)
 		_, ok := auth.(*gitssh.PublicKeys)
-		require.True(t, ok)
+		require.True(t, ok, "expected SSH auth for git:// URL")
 	})
 
 	t.Run("SSH/GitUsername", func(t *testing.T) {

go.mod

@@ -13,7 +13,6 @@ require (
 	cdr.dev/slog v1.6.2-0.20240126064726-20367d4aede6
 	github.com/GoogleContainerTools/kaniko v1.9.2
 	github.com/breml/rootcerts v0.2.10
-	github.com/chainguard-dev/git-urls v1.0.2
 	github.com/coder/coder/v2 v2.10.1-0.20240704130443-c2d44d16a352
 	github.com/coder/retry v1.5.1
 	github.com/coder/serpent v0.8.0

go.sum

@@ -152,8 +152,6 @@ github.com/certifi/gocertifi v0.0.0-20200922220541-2c3bb06c6054/go.mod h1:sGbDF6
 github.com/cespare/xxhash/v2 v2.1.1/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
 github.com/cespare/xxhash/v2 v2.2.0 h1:DC2CZ1Ep5Y4k3ZQ899DldepgrayRUGE6BBZ/cd9Cj44=
 github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
-github.com/chainguard-dev/git-urls v1.0.2 h1:pSpT7ifrpc5X55n4aTTm7FFUE+ZQHKiqpiwNkJrVcKQ=
-github.com/chainguard-dev/git-urls v1.0.2/go.mod h1:rbGgj10OS7UgZlbzdUQIQpT0k/D4+An04HJY7Ol+Y/o=
 github.com/charmbracelet/lipgloss v0.8.0 h1:IS00fk4XAHcf8uZKc3eHeMUTCxUH6NkaTrdyCQk84RU=
 github.com/charmbracelet/lipgloss v0.8.0/go.mod h1:p4eYUZZJ/0oXTuCQKFF8mqyKCz0ja6y+7DniDDw5KKU=
 github.com/chenzhuoyu/base64x v0.0.0-20230717121745-296ad89f973d h1:77cEq6EriyTZ0g/qfRdp61a3Uu/AWrgIq2s0ClJV1g0=

integration/integration_test.go

@@ -434,14 +434,16 @@ func TestGitSSHAuth(t *testing.T) {
 		_ = gittest.NewRepo(t, srvFS, gittest.Commit(t, "Dockerfile", "FROM "+testImageAlpine, "Initial commit"))
 		tr := gittest.NewServerSSH(t, srvFS, signer.PublicKey())
 
-		_, err = runEnvbuilder(t, runOpts{env: []string{
+		ctr, err := runEnvbuilder(t, runOpts{env: []string{
 			envbuilderEnv("DOCKERFILE_PATH", "Dockerfile"),
-			envbuilderEnv("GIT_URL", tr.String()+"."),
+			envbuilderEnv("GIT_URL", tr.String()),
 			envbuilderEnv("GIT_SSH_PRIVATE_KEY_BASE64", base64Key),
 		}})
-		// TODO: Ensure it actually clones but this does mean we have
-		// successfully authenticated.
-		require.ErrorContains(t, err, "repository not found")
+		require.NoError(t, err)
+		dockerFilePath := execContainer(t, ctr, "find /workspaces -name Dockerfile")
+		require.NotEmpty(t, dockerFilePath)
+		dockerFile := execContainer(t, ctr, "cat "+dockerFilePath)
+		require.Contains(t, dockerFile, testImageAlpine)
 	})
 
 	t.Run("Base64/Failure", func(t *testing.T) {

internal/ebutil/giturls.go

@@ -0,0 +1,61 @@
+package ebutil
+
+import (
+	"fmt"
+	"net/url"
+	"strings"
+
+	gittransport "github.com/go-git/go-git/v5/plumbing/transport"
+)
+
+type ParsedURL struct {
+	Protocol  string
+	User      string
+	Password  string
+	Host      string
+	Port      int
+	Path      string
+	Reference string
+}
+
+// ParseRepoURL parses the given repository URL into its components.
+// We used to use chainguard-dev/git-urls for this, but its behaviour
+// diverges from the go-git URL parser. To ensure consistency, we now
+// use go-git directly with some tweaks.
+func ParseRepoURL(repoURL string) (*ParsedURL, error) {
+	repoURL = fixupScheme(repoURL, "ssh://")
+	repoURL = fixupScheme(repoURL, "git://")
+	repoURL = fixupScheme(repoURL, "git+ssh://")
+	parsed, err := gittransport.NewEndpoint(repoURL)
+	if err != nil {
+		return nil, fmt.Errorf("parse repo url %q: %w", repoURL, err)
+	}
+	// Trim #reference from path
+	var reference string
+	if len(parsed.Path) > 0 { // annoyingly, strings.Index returns 0 if len(s) == 0
+		if idx := strings.Index(parsed.Path, "#"); idx > -1 {
+			reference = parsed.Path[idx+1:]
+			parsed.Path = parsed.Path[:idx]
+		}
+	}
+	return &ParsedURL{
+		Protocol:  parsed.Protocol,
+		User:      parsed.User,
+		Password:  parsed.Password,
+		Host:      parsed.Host,
+		Port:      parsed.Port,
+		Path:      parsed.Path,
+		Reference: reference,
+	}, nil
+}
+
+func fixupScheme(repoURL, scheme string) string {
+	// go-git tries to handle protocol:// URLs with url.Parse. This fails
+	// in the case of e.g. (ssh|git)://git@host:user/path.git
+	if cut, found := strings.CutPrefix(repoURL, scheme); found {
+		if _, err := url.Parse(repoURL); err != nil && strings.Contains(err.Error(), "invalid port") {
+			return cut
+		}
+	}
+	return repoURL
+}

log/coder_internal_test.go

@@ -9,6 +9,7 @@ import (
 	"net/http/httptest"
 	"net/url"
 	"sync"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -198,16 +199,26 @@ func TestCoder(t *testing.T) {
 		defer cancel()
 
 		token := uuid.NewString()
-		done := make(chan struct{})
-		handlerSend := make(chan int)
+		var calls atomic.Int64
 		handler := func(w http.ResponseWriter, r *http.Request) {
-			t.Logf("test handler: %s", r.URL.Path)
 			if r.URL.Path == "/api/v2/buildinfo" {
 				w.Header().Set("Content-Type", "application/json")
 				_, _ = w.Write([]byte(`{"version": "v2.9.0"}`))
 				return
 			}
-			code := <-handlerSend
+			n := calls.Add(1)
+			t.Logf("test handler: %s call %d", r.URL.Path, n)
+			var code int
+			switch n {
+			// The first two calls should fail with a 401.
+			case 1, 2:
+				code = http.StatusUnauthorized
+			case 3:
+				code = http.StatusOK
+			default:
+				cancel()
+				return
+			}
 			t.Logf("test handler response: %d", code)
 			w.WriteHeader(code)
 		}
@@ -216,25 +227,10 @@ func TestCoder(t *testing.T) {
 
 		u, err := url.Parse(srv.URL)
 		require.NoError(t, err)
-		var connectError error
-		go func() {
-			defer close(handlerSend)
-			defer close(done)
-			_, _, connectError = Coder(ctx, u, token)
-		}()
-
-		// Initial: unauthorized
-		handlerSend <- http.StatusUnauthorized
-		// 2nd try: still unauthorized
-		handlerSend <- http.StatusUnauthorized
-		// 3rd try: authorized
-		handlerSend <- http.StatusOK
-
-		cancel()
-
-		<-done
-		require.ErrorContains(t, connectError, "failed to WebSocket dial")
+		_, _, connectError := Coder(ctx, u, token)
 		require.ErrorIs(t, connectError, context.Canceled)
+		// Should have retried at least twice.
+		require.Greater(t, calls.Load(), int64(2))
 	})
 }
 

options/defaults.go

@@ -7,8 +7,8 @@ import (
 
 	"github.com/go-git/go-billy/v5/osfs"
 
-	giturls "github.com/chainguard-dev/git-urls"
 	"github.com/coder/envbuilder/internal/chmodfs"
+	"github.com/coder/envbuilder/internal/ebutil"
 	"github.com/coder/envbuilder/internal/workingdir"
 )
 
@@ -22,7 +22,7 @@ func DefaultWorkspaceFolder(workspacesFolder, repoURL string) string {
 	if repoURL == "" {
 		return emptyWorkspaceDir
 	}
-	parsed, err := giturls.Parse(repoURL)
+	parsed, err := ebutil.ParseRepoURL(repoURL)
 	if err != nil {
 		return emptyWorkspaceDir
 	}

options/defaults_test.go

@@ -33,6 +33,24 @@ func TestDefaultWorkspaceFolder(t *testing.T) {
 			gitURL:   "git@github.com:coder/envbuilder.git",
 			expected: "/workspaces/envbuilder",
 		},
+		{
+			name:     "SSH with prefix",
+			baseDir:  "/workspaces",
+			gitURL:   "ssh://git@github.com:coder/envbuilder.git",
+			expected: "/workspaces/envbuilder",
+		},
+		{
+			name:     "Git protocol is SSH",
+			baseDir:  "/workspaces",
+			gitURL:   "git://github.com/coder/envbuilder.git",
+			expected: "/workspaces/envbuilder",
+		},
+		{
+			name:     "Git+SSH protocol is SSH",
+			baseDir:  "/workspaces",
+			gitURL:   "git+ssh://github.com/coder/envbuilder.git",
+			expected: "/workspaces/envbuilder",
+		},
 		{
 			name:     "username and password",
 			baseDir:  "/workspaces",

testutil/gittest/gittest.go

@@ -165,8 +165,9 @@ func NewServerSSH(t *testing.T, fs billy.Filesystem, pubkeys ...gossh.PublicKey)
 
 	addr, ok := l.Addr().(*net.TCPAddr)
 	require.True(t, ok)
-	tr, err := transport.NewEndpoint(fmt.Sprintf("ssh://git@%s:%d/", addr.IP, addr.Port))
+	tr, err := transport.NewEndpoint(fmt.Sprintf("ssh://git@%s:%d%s", addr.IP, addr.Port, fs.Root()))
 	require.NoError(t, err)
+	t.Logf("git-ssh url: %s", tr.String())
 	return tr
 }