diff --git a/docs/git-auth.md b/docs/git-auth.md index e2b8cd28..c5a7303d 100644 --- a/docs/git-auth.md +++ b/docs/git-auth.md @@ -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 | + +# Authentication Methods Two methods of authentication are supported: diff --git a/git/git.go b/git/git.go index efcffa91..320b40c4 100644 --- a/git/git.go +++ b/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: 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, @@ -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!") @@ -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 diff --git a/git/git_test.go b/git/git_test.go index 0da5a163..c6422897 100644 --- a/git/git_test.go +++ b/git/git_test.go @@ -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" @@ -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) { @@ -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) @@ -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{ @@ -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 @@ -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) @@ -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)) }) }) } @@ -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()) @@ -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")) }) t.Run("AuthFailure", func(t *testing.T) { @@ -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) { @@ -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) @@ -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) @@ -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 @@ -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 { diff --git a/go.mod b/go.mod index befe817b..5dc54dda 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 5f55fc6f..cc807407 100644 --- a/go.sum +++ b/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= diff --git a/integration/integration_test.go b/integration/integration_test.go index 913ab567..65f4902e 100644 --- a/integration/integration_test.go +++ b/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) { diff --git a/internal/ebutil/giturls.go b/internal/ebutil/giturls.go new file mode 100644 index 00000000..990903a6 --- /dev/null +++ b/internal/ebutil/giturls.go @@ -0,0 +1,57 @@ +package ebutil + +import ( + "fmt" + "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 + Password string + Host string + Port int + Path string + Reference string + Cleaned 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. +func ParseRepoURL(repoURL string) (*ParsedURL, error) { + // Trim leading and trailing whitespace + repoURL = strings.TrimSpace(repoURL) + // Trim #reference from path + var reference string + 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, + User: parsed.User, + Password: parsed.Password, + Host: parsed.Host, + Port: parsed.Port, + Path: parsed.Path, + Reference: reference, + Cleaned: repoURL, + }, nil +} diff --git a/log/coder_internal_test.go b/log/coder_internal_test.go index 8b8bb632..77522e44 100644 --- a/log/coder_internal_test.go +++ b/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)) }) } diff --git a/options/defaults.go b/options/defaults.go index ee2543f7..c7aca9f3 100644 --- a/options/defaults.go +++ b/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 } diff --git a/options/defaults_test.go b/options/defaults_test.go index 91dac3bd..edc2bf3c 100644 --- a/options/defaults_test.go +++ b/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 scheme", + 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", + baseDir: "/workspaces", + gitURL: "git+ssh://github.com/coder/envbuilder.git", + expected: "/workspaces/envbuilder", + }, { name: "username and password", baseDir: "/workspaces", diff --git a/testutil/gittest/gittest.go b/testutil/gittest/gittest.go index f3d5f1d3..1a0e2424 100644 --- a/testutil/gittest/gittest.go +++ b/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 }