Skip to content

fix: improve git URL parsing#486

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
johnstcn merged 10 commits into from
Jan 26, 2026
Merged

fix: improve git URL parsing #486

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
83c4567
chore: use go-git git url parser
johnstcn Jan 22, 2026
5298b6f
chore: fix gittest ssh repo path
johnstcn Jan 22, 2026
61c6d08
chore: fix flaky test
johnstcn Jan 22, 2026
8b28b71
chore: go mod tidy
johnstcn Jan 22, 2026
7549a40
this url.Parse is actually OK
johnstcn Jan 22, 2026
4ad93b8
update docs to reference git auth
johnstcn Jan 23, 2026
85928c9
only accept valid git URL formats
johnstcn Jan 23, 2026
3618988
improve whitespace handling
johnstcn Jan 23, 2026
0993246
add ssh/git examples
johnstcn Jan 26, 2026
d5a3784
add scp-like example
johnstcn Jan 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion docs/git-auth.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,27 @@
# Git Authentication
# Supported URL Formats

Envbuilder supports three distinct types of Git URLs:

1) Valid URLs with scheme (e.g. `https://user:password@host.tld:12345/path/to/repo`)
2) SCP-like URLs (e.g. `git@host.tld:path/to/repo.git`)
3) Filesystem URLs (require the `git` executable to be available)

Based on the type of URL, one of two authentication methods will be used:

| Git URL format | GIT_USERNAME | GIT_PASSWORD | Auth Method |
| ------------------------|--------------|--------------|-------------|
| https?://host.tld/repo | Not Set | Not Set | None |
| https?://host.tld/repo | Not Set | Set | HTTP Basic |
| https?://host.tld/repo | Set | Not Set | HTTP Basic |
| https?://host.tld/repo | Set | Set | HTTP Basic |
| ssh://host.tld/repo | - | - | SSH |
| git://host.tld/repo | - | - | SSH |
| file://path/to/repo | - | - | None |
| path/to/repo | - | - | None |
| user@host.tld:path/repo | - | - | SSH |
| All other formats | - | - | SSH |
Comment thread
johnstcn marked this conversation as resolved.

# Authentication Methods

Two methods of authentication are supported:

Expand Down
29 changes: 15 additions & 14 deletions git/git.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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"
Expand DownExpand Up@@ -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.
Expand DownExpand Up@@ -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)
Expand All@@ -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: parsed.Cleaned,
Auth: opts.RepoAuth,
Progress: opts.Progress,
ReferenceName: plumbing.ReferenceName(reference),
ReferenceName: plumbing.ReferenceName(parsed.Reference),
InsecureSkipTLS: opts.Insecure,
Depth: opts.Depth,
SingleBranch: opts.SingleBranch,
Expand DownExpand Up@@ -245,18 +241,23 @@ 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. git@host.tld: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!")
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!")
Expand All@@ -272,7 +273,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
Expand Down
75 changes: 58 additions & 17 deletions git/git_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,13 +4,12 @@ import (
"context"
"crypto/ed25519"
"encoding/base64"
"fmt"
"io"
"net/http/httptest"
"net/url"
"os"
"path/filepath"
"regexp"
"strings"
"testing"

"github.com/coder/envbuilder/git"
Expand DownExpand Up@@ -76,6 +75,18 @@ func TestCloneRepo(t *testing.T) {
password: "",
expectClone: true,
},
{
name: "whitespace in URL",
mungeURL: func(u *string) {
if u == nil {
t.Errorf("expected non-nil URL")
return
}
*u = " " + *u + " "
t.Logf("munged URL: %q", *u)
},
expectClone: true,
},
} {
tc := tc
t.Run(tc.name, func(t *testing.T) {
Expand All@@ -87,6 +98,9 @@ func TestCloneRepo(t *testing.T) {
_ = gittest.NewRepo(t, srvFS, gittest.Commit(t, "README.md", "Hello, world!", "Wow!"))
authMW := mwtest.BasicAuthMW(tc.srvUsername, tc.srvPassword)
srv := httptest.NewServer(authMW(gittest.NewServer(srvFS)))
if tc.mungeURL != nil {
tc.mungeURL(&srv.URL)
}
clientFS := memfs.New()
// A repo already exists!
_ = gittest.NewRepo(t, clientFS)
Expand All@@ -106,6 +120,9 @@ func TestCloneRepo(t *testing.T) {
_ = gittest.NewRepo(t, srvFS, gittest.Commit(t, "README.md", "Hello, world!", "Wow!"))
authMW := mwtest.BasicAuthMW(tc.srvUsername, tc.srvPassword)
srv := httptest.NewServer(authMW(gittest.NewServer(srvFS)))
if tc.mungeURL != nil {
tc.mungeURL(&srv.URL)
}
clientFS := memfs.New()

cloned, err := git.CloneRepo(context.Background(), t.Logf, git.CloneRepoOptions{
Expand All@@ -129,7 +146,7 @@ func TestCloneRepo(t *testing.T) {
require.Equal(t, "Hello, world!", readme)
gitConfig := mustRead(t, clientFS, "/workspace/.git/config")
// Ensure we do not modify the git URL that folks pass in.
require.Regexp(t, fmt.Sprintf(`(?m)^\s+url\s+=\s+%s\s*$`, regexp.QuoteMeta(srv.URL)), gitConfig)
require.Contains(t, gitConfig, strings.TrimSpace(srv.URL))
})

// In-URL-style auth e.g. http://user:password@host:port
Expand All@@ -139,15 +156,19 @@ func TestCloneRepo(t *testing.T) {
_ = gittest.NewRepo(t, srvFS, gittest.Commit(t, "README.md", "Hello, world!", "Wow!"))
authMW := mwtest.BasicAuthMW(tc.srvUsername, tc.srvPassword)
srv := httptest.NewServer(authMW(gittest.NewServer(srvFS)))

authURL, err := url.Parse(srv.URL)
require.NoError(t, err)
authURL.User = url.UserPassword(tc.username, tc.password)
cloneURL := authURL.String()
if tc.mungeURL != nil {
tc.mungeURL(&cloneURL)
}

clientFS := memfs.New()

cloned, err := git.CloneRepo(context.Background(), t.Logf, git.CloneRepoOptions{
Path: "/workspace",
RepoURL: authURL.String(),
RepoURL: cloneURL,
Storage: clientFS,
})
require.Equal(t, tc.expectClone, cloned)
Expand All@@ -162,7 +183,7 @@ func TestCloneRepo(t *testing.T) {
require.Equal(t, "Hello, world!", readme)
gitConfig := mustRead(t, clientFS, "/workspace/.git/config")
// Ensure we do not modify the git URL that folks pass in.
require.Regexp(t, fmt.Sprintf(`(?m)^\s+url\s+=\s+%s\s*$`, regexp.QuoteMeta(authURL.String())), gitConfig)
require.Contains(t, gitConfig, strings.TrimSpace(cloneURL))
})
})
}
Expand DownExpand Up@@ -238,10 +259,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())

Expand All@@ -264,10 +284,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"))
Comment on lines +287 to +289

@johnstcnjohnstcnJan 22, 2026

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

})

t.Run("AuthFailure", func(t *testing.T) {
Expand DownExpand Up@@ -399,12 +418,12 @@ func TestSetupRepoAuth(t *testing.T) {
// Anything that is not https:// or http:// is treated as SSH.
kPath := writeTestPrivateKey(t)
opts := &options.Options{
GitURL: "git://git@host.tld:repo/path",
GitURL: "git://git@host.tld:12345/path",
GitSSHPrivateKeyPath: kPath,
}
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) {
Expand All@@ -422,7 +441,7 @@ func TestSetupRepoAuth(t *testing.T) {
t.Run("SSH/PrivateKey", func(t *testing.T) {
kPath := writeTestPrivateKey(t)
opts := &options.Options{
GitURL: "ssh://git@host.tld:repo/path",
GitURL: "ssh://git@host.tld/repo/path",
GitSSHPrivateKeyPath: kPath,
}
auth := git.SetupRepoAuth(t.Logf, opts)
Expand All@@ -436,7 +455,7 @@ func TestSetupRepoAuth(t *testing.T) {

t.Run("SSH/Base64PrivateKey", func(t *testing.T) {
opts := &options.Options{
GitURL: "ssh://git@host.tld:repo/path",
GitURL: "ssh://git@host.tld/repo/path",
GitSSHPrivateKeyBase64: base64EncodeTestPrivateKey(),
}
auth := git.SetupRepoAuth(t.Logf, opts)
Expand All@@ -452,7 +471,7 @@ func TestSetupRepoAuth(t *testing.T) {

t.Run("SSH/NoAuthMethods", func(t *testing.T) {
opts := &options.Options{
GitURL: "ssh://git@host.tld:repo/path",
GitURL: "git@host.tld:repo/path",
}
auth := git.SetupRepoAuth(t.Logf, opts)
require.Nil(t, auth) // TODO: actually test SSH_AUTH_SOCK
Expand DownExpand Up@@ -481,6 +500,28 @@ func TestSetupRepoAuth(t *testing.T) {
auth := git.SetupRepoAuth(t.Logf, opts)
require.Nil(t, auth)
})

t.Run("Whitespace", func(t *testing.T) {
kPath := writeTestPrivateKey(t)
opts := &options.Options{
GitURL: "ssh://git@host.tld/repo path",
GitSSHPrivateKeyPath: kPath,
}
auth := git.SetupRepoAuth(t.Logf, opts)
_, ok := auth.(*gitssh.PublicKeys)
require.True(t, ok)
})

t.Run("LeadingTrailingWhitespace", func(t *testing.T) {
kPath := writeTestPrivateKey(t)
opts := &options.Options{
GitURL: " ssh://git@host.tld/repo/path ",
GitSSHPrivateKeyPath: kPath,
}
auth := git.SetupRepoAuth(t.Logf, opts)
_, ok := auth.(*gitssh.PublicKeys)
require.True(t, ok)
})
}

func mustRead(t *testing.T, fs billy.Filesystem, path string) string {
Expand Down
1 change: 0 additions & 1 deletion go.mod
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand Down
2 changes: 0 additions & 2 deletions go.sum
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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=
Expand Down
12 changes: 7 additions & 5 deletions integration/integration_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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)
Comment on lines +442 to +446

@johnstcnjohnstcnJan 22, 2026

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

})

t.Run("Base64/Failure", func(t *testing.T) {
Expand Down
Loading
Loading