only accept valid git URL formats

Author: johnstcn

git/git.go

@@ -241,6 +241,11 @@ func LogHostKeyCallback(logger func(string, ...any)) gossh.HostKeyCallback {
 // If SSH_KNOWN_HOSTS is not set, the SSH auth method will be configured
 // to accept and log all host keys. Otherwise, host key checking will be
 // performed as usual.
+//
+// Git URL formats may only consist of the following:
+//  1. A valid URL with a scheme
+//  2. An SCP-like URL (e.g. [email protected]:path/to/repo.git)
+//  3. Local filesystem paths (require `git` executable)
 func SetupRepoAuth(logf func(string, ...any), options *options.Options) transport.AuthMethod {
 	if options.GitURL == "" {
 		logf("❔ No Git URL supplied!")

git/git_test.go

@@ -397,7 +397,7 @@ func TestSetupRepoAuth(t *testing.T) {
 		// Anything that is not https:// or http:// is treated as SSH.
 		kPath := writeTestPrivateKey(t)
 		opts := &options.Options{
-			GitURL:               "git://[email protected]:repo/path",
+			GitURL:               "git://[email protected]:12345/path",
 			GitSSHPrivateKeyPath: kPath,
 		}
 		auth := git.SetupRepoAuth(t.Logf, opts)
@@ -420,7 +420,7 @@ func TestSetupRepoAuth(t *testing.T) {
 	t.Run("SSH/PrivateKey", func(t *testing.T) {
 		kPath := writeTestPrivateKey(t)
 		opts := &options.Options{
-			GitURL:               "ssh://[email protected]:repo/path",
+			GitURL:               "ssh://[email protected]/repo/path",
 			GitSSHPrivateKeyPath: kPath,
 		}
 		auth := git.SetupRepoAuth(t.Logf, opts)
@@ -434,7 +434,7 @@ func TestSetupRepoAuth(t *testing.T) {
 
 	t.Run("SSH/Base64PrivateKey", func(t *testing.T) {
 		opts := &options.Options{
-			GitURL:                 "ssh://[email protected]:repo/path",
+			GitURL:                 "ssh://[email protected]/repo/path",
 			GitSSHPrivateKeyBase64: base64EncodeTestPrivateKey(),
 		}
 		auth := git.SetupRepoAuth(t.Logf, opts)
@@ -450,7 +450,7 @@ func TestSetupRepoAuth(t *testing.T) {
 
 	t.Run("SSH/NoAuthMethods", func(t *testing.T) {
 		opts := &options.Options{
-			GitURL: "ssh://[email protected]:repo/path",
+			GitURL: "[email protected]:repo/path",
 		}
 		auth := git.SetupRepoAuth(t.Logf, opts)
 		require.Nil(t, auth) // TODO: actually test SSH_AUTH_SOCK

internal/ebutil/giturls.go

@@ -2,12 +2,20 @@ package ebutil
 
 import (
 	"fmt"
-	"net/url"
 	"strings"
 
 	gittransport "github.com/go-git/go-git/v5/plumbing/transport"
 )
 
+type InvalidRepoURLError struct {
+	repoURL string
+	inner   error
+}
+
+func (e *InvalidRepoURLError) Error() string {
+	return fmt.Sprintf("invalid repository URL %q, please see https://github.com/coder/envbuilder/blob/main/docs/git-auth.md for supported formats: %v", e.repoURL, e.inner)
+}
+
 type ParsedURL struct {
 	Protocol  string
 	User      string
@@ -21,22 +29,17 @@ type ParsedURL struct {
 // 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.
+// use go-git directly.
 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]
-		}
+	if idx := strings.Index(repoURL, "#"); idx > -1 {
+		reference = repoURL[idx+1:]
+		repoURL = repoURL[:idx]
+	}
+	parsed, err := gittransport.NewEndpoint(repoURL)
+	if err != nil {
+		return nil, &InvalidRepoURLError{repoURL: repoURL, inner: err}
 	}
 	return &ParsedURL{
 		Protocol:  parsed.Protocol,
@@ -48,14 +51,3 @@ func ParseRepoURL(repoURL string) (*ParsedURL, error) {
 		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
-}

options/defaults_test.go

@@ -34,9 +34,9 @@ func TestDefaultWorkspaceFolder(t *testing.T) {
 			expected: "/workspaces/envbuilder",
 		},
 		{
-			name:     "SSH with prefix",
+			name:     "SSH with scheme",
 			baseDir:  "/workspaces",
-			gitURL:   "ssh://[email protected]:coder/envbuilder.git",
+			gitURL:   "ssh://[email protected]/coder/envbuilder.git",
 			expected: "/workspaces/envbuilder",
 		},
 		{
@@ -46,7 +46,7 @@ func TestDefaultWorkspaceFolder(t *testing.T) {
 			expected: "/workspaces/envbuilder",
 		},
 		{
-			name:     "Git+SSH protocol is SSH",
+			name:     "Git+SSH",
 			baseDir:  "/workspaces",
 			gitURL:   "git+ssh://github.com/coder/envbuilder.git",
 			expected: "/workspaces/envbuilder",