diff --git a/source/git/gitsource.go b/source/git/gitsource.go index 3d1bfe21f4d9..9245008aa852 100644 --- a/source/git/gitsource.go +++ b/source/git/gitsource.go @@ -129,7 +129,11 @@ func (gs *gitSource) mountRemote(ctx context.Context, remote string, auth []stri }() if initializeRepo { - if _, err := gitWithinDir(ctx, dir, "", "", "", auth, "init", "--bare"); err != nil { + // Explicitly set the Git config 'init.defaultBranch' to the + // implied default to suppress "hint:" output about not having a + // default initial branch name set which otherwise spams unit + // test logs. + if _, err := gitWithinDir(ctx, dir, "", "", "", auth, "-c", "init.defaultBranch=master", "init", "--bare"); err != nil { return "", nil, errors.Wrapf(err, "failed to init repo at %s", dir) } @@ -485,11 +489,14 @@ func (gs *gitSourceHandler) Snapshot(ctx context.Context, g session.Group) (out if err := os.MkdirAll(checkoutDir, 0711); err != nil { return nil, err } - _, err = gitWithinDir(ctx, checkoutDirGit, "", sock, knownHosts, nil, "init") + _, err = gitWithinDir(ctx, checkoutDirGit, "", sock, knownHosts, nil, "-c", "init.defaultBranch=master", "init") if err != nil { return nil, err } - _, err = gitWithinDir(ctx, checkoutDirGit, "", sock, knownHosts, nil, "remote", "add", "origin", gitDir) + // Defense-in-depth: clone using the file protocol to disable local-clone + // optimizations which can be abused on some versions of Git to copy unintended + // host files into the build context. + _, err = gitWithinDir(ctx, checkoutDirGit, "", sock, knownHosts, nil, "remote", "add", "origin", "file://"+gitDir) if err != nil { return nil, err } @@ -591,6 +598,7 @@ func git(ctx context.Context, dir, sshAuthSock, knownHosts string, args ...strin stdout, stderr := logs.NewLogStreams(ctx, false) defer stdout.Close() defer stderr.Close() + args = append([]string{"-c", "protocol.file.allow=user"}, args...) // Block sneaky repositories from using repos from the filesystem as submodules. cmd := exec.Command("git", args...) cmd.Dir = dir // some commands like submodule require this buf := bytes.NewBuffer(nil) @@ -603,6 +611,8 @@ func git(ctx context.Context, dir, sshAuthSock, knownHosts string, args ...strin "GIT_TERMINAL_PROMPT=0", "GIT_SSH_COMMAND=" + getGitSSHCommand(knownHosts), // "GIT_TRACE=1", + "GIT_CONFIG_NOSYSTEM=1", // Disable reading from system gitconfig. + "HOME=/dev/null", // Disable reading from user gitconfig. } if sshAuthSock != "" { cmd.Env = append(cmd.Env, "SSH_AUTH_SOCK="+sshAuthSock) diff --git a/source/git/gitsource_test.go b/source/git/gitsource_test.go index 635d05757fff..14892d1012f6 100644 --- a/source/git/gitsource_test.go +++ b/source/git/gitsource_test.go @@ -1,12 +1,17 @@ package git import ( + "bytes" "context" "io/ioutil" + "net/http" + "net/http/cgi" + "net/http/httptest" "os" "os/exec" "path/filepath" "runtime" + "strconv" "strings" "testing" @@ -17,10 +22,12 @@ import ( "github.com/containerd/containerd/snapshots/native" "github.com/moby/buildkit/cache" "github.com/moby/buildkit/cache/metadata" + "github.com/moby/buildkit/client" "github.com/moby/buildkit/snapshot" containerdsnapshot "github.com/moby/buildkit/snapshot/containerd" "github.com/moby/buildkit/source" "github.com/moby/buildkit/util/leaseutil" + "github.com/moby/buildkit/util/progress" "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -40,7 +47,7 @@ func testRepeatedFetch(t *testing.T, keepGitDir bool) { } t.Parallel() - ctx := context.TODO() + ctx := logProgressStreams(context.Background(), t) tmpdir, err := ioutil.TempDir("", "buildkit-state") require.NoError(t, err) @@ -52,10 +59,10 @@ func testRepeatedFetch(t *testing.T, keepGitDir bool) { require.NoError(t, err) defer os.RemoveAll(repodir) - repodir, err = setupGitRepo(repodir) + repo := setupGitRepo(t, repodir) require.NoError(t, err) - id := &source.GitIdentifier{Remote: repodir, KeepGitDir: keepGitDir} + id := &source.GitIdentifier{Remote: repo.mainURL, KeepGitDir: keepGitDir} g, err := gs.Resolve(ctx, id, nil, nil) require.NoError(t, err) @@ -97,7 +104,7 @@ func testRepeatedFetch(t *testing.T, keepGitDir bool) { require.True(t, errors.Is(err, os.ErrNotExist)) // second fetch returns same dir - id = &source.GitIdentifier{Remote: repodir, Ref: "master", KeepGitDir: keepGitDir} + id = &source.GitIdentifier{Remote: repo.mainURL, Ref: "master", KeepGitDir: keepGitDir} g, err = gs.Resolve(ctx, id, nil, nil) require.NoError(t, err) @@ -113,7 +120,7 @@ func testRepeatedFetch(t *testing.T, keepGitDir bool) { require.Equal(t, ref1.ID(), ref2.ID()) - id = &source.GitIdentifier{Remote: repodir, Ref: "feature", KeepGitDir: keepGitDir} + id = &source.GitIdentifier{Remote: repo.mainURL, Ref: "feature", KeepGitDir: keepGitDir} g, err = gs.Resolve(ctx, id, nil, nil) require.NoError(t, err) @@ -159,6 +166,7 @@ func testFetchBySHA(t *testing.T, keepGitDir bool) { t.Parallel() ctx := namespaces.WithNamespace(context.Background(), "buildkit-test") + ctx = logProgressStreams(ctx, t) tmpdir, err := ioutil.TempDir("", "buildkit-state") require.NoError(t, err) @@ -170,11 +178,10 @@ func testFetchBySHA(t *testing.T, keepGitDir bool) { require.NoError(t, err) defer os.RemoveAll(repodir) - repodir, err = setupGitRepo(repodir) - require.NoError(t, err) + repo := setupGitRepo(t, repodir) cmd := exec.Command("git", "rev-parse", "feature") - cmd.Dir = repodir + cmd.Dir = repo.mainPath out, err := cmd.Output() require.NoError(t, err) @@ -182,7 +189,7 @@ func testFetchBySHA(t *testing.T, keepGitDir bool) { sha := strings.TrimSpace(string(out)) require.Equal(t, 40, len(sha)) - id := &source.GitIdentifier{Remote: repodir, Ref: sha, KeepGitDir: keepGitDir} + id := &source.GitIdentifier{Remote: repo.mainURL, Ref: sha, KeepGitDir: keepGitDir} g, err := gs.Resolve(ctx, id, nil, nil) require.NoError(t, err) @@ -236,6 +243,7 @@ func testMultipleRepos(t *testing.T, keepGitDir bool) { t.Parallel() ctx := namespaces.WithNamespace(context.Background(), "buildkit-test") + ctx = logProgressStreams(ctx, t) tmpdir, err := ioutil.TempDir("", "buildkit-state") require.NoError(t, err) @@ -247,25 +255,24 @@ func testMultipleRepos(t *testing.T, keepGitDir bool) { require.NoError(t, err) defer os.RemoveAll(repodir) - repodir, err = setupGitRepo(repodir) - require.NoError(t, err) + repo := setupGitRepo(t, repodir) repodir2, err := ioutil.TempDir("", "buildkit-gitsource") require.NoError(t, err) defer os.RemoveAll(repodir2) - err = runShell(repodir2, - "git init", + runShell(t, repodir2, + "git -c init.defaultBranch=master init", "git config --local user.email test", "git config --local user.name test", "echo xyz > xyz", "git add xyz", "git commit -m initial", ) - require.NoError(t, err) + repoURL2 := serveGitRepo(t, repodir2) - id := &source.GitIdentifier{Remote: repodir, KeepGitDir: keepGitDir} - id2 := &source.GitIdentifier{Remote: repodir2, KeepGitDir: keepGitDir} + id := &source.GitIdentifier{Remote: repo.mainURL, KeepGitDir: keepGitDir} + id2 := &source.GitIdentifier{Remote: repoURL2, KeepGitDir: keepGitDir} g, err := gs.Resolve(ctx, id, nil, nil) require.NoError(t, err) @@ -358,30 +365,33 @@ func setupGitSource(t *testing.T, tmpdir string) source.Source { return gs } -func setupGitRepo(dir string) (string, error) { - subPath := filepath.Join(dir, "sub") - mainPath := filepath.Join(dir, "main") - - if err := os.MkdirAll(subPath, 0700); err != nil { - return "", err - } +type gitRepoFixture struct { + mainPath, subPath string // Filesystem paths to the respective repos + mainURL, subURL string // HTTP URLs for the respective repos +} - if err := os.MkdirAll(mainPath, 0700); err != nil { - return "", err +func setupGitRepo(t *testing.T, dir string) gitRepoFixture { + t.Helper() + srv := serveGitRepo(t, dir) + fixture := gitRepoFixture{ + subPath: filepath.Join(dir, "sub"), + subURL: srv + "/sub", + mainPath: filepath.Join(dir, "main"), + mainURL: srv + "/main", } + require.NoError(t, os.MkdirAll(fixture.subPath, 0700)) + require.NoError(t, os.MkdirAll(fixture.mainPath, 0700)) - if err := runShell(filepath.Join(dir, "sub"), - "git init", + runShell(t, fixture.subPath, + "git -c init.defaultBranch=master init", "git config --local user.email test", "git config --local user.name test", "echo subcontents > subfile", "git add subfile", "git commit -m initial", - ); err != nil { - return "", err - } - if err := runShell(filepath.Join(dir, "main"), - "git init", + ) + runShell(t, fixture.mainPath, + "git -c init.defaultBranch=master init", "git config --local user.email test", "git config --local user.name test", "echo foo > abc", @@ -394,16 +404,58 @@ func setupGitRepo(dir string) (string, error) { "echo baz > ghi", "git add ghi", "git commit -m feature", - "git submodule add "+subPath+" sub", + "git submodule add "+fixture.subURL+" sub", "git add -A", "git commit -m withsub", - ); err != nil { - return "", err - } - return mainPath, nil + "git checkout master", + ) + return fixture } -func runShell(dir string, cmds ...string) error { +func serveGitRepo(t *testing.T, root string) string { + t.Helper() + gitpath, err := exec.LookPath("git") + require.NoError(t, err) + gitversion, _ := exec.Command(gitpath, "version").CombinedOutput() + t.Logf("%s", gitversion) // E.g. "git version 2.30.2" + + // Serve all repositories under root using the Smart HTTP protocol so + // they can be cloned as we explicitly disable the file protocol. + // (Another option would be to use `git daemon` and the Git protocol, + // but that listens on a fixed port number which is a recipe for + // disaster in CI. Funnily enough, `git daemon --port=0` works but there + // is no easy way to discover which port got picked!) + + githttp := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var logs bytes.Buffer + (&cgi.Handler{ + Path: gitpath, + Args: []string{"http-backend"}, + Dir: root, + Env: []string{ + "GIT_PROJECT_ROOT=" + root, + "GIT_HTTP_EXPORT_ALL=1", + }, + Stderr: &logs, + }).ServeHTTP(w, r) + if logs.Len() == 0 { + return + } + for { + line, err := logs.ReadString('\n') + t.Log("git-http-backend: " + line) + if err != nil { + break + } + } + }) + server := httptest.NewServer(&githttp) + t.Cleanup(server.Close) + return server.URL +} + +func runShell(t *testing.T, dir string, cmds ...string) { + t.Helper() for _, args := range cmds { var cmd *exec.Cmd if runtime.GOOS == "windows" { @@ -412,9 +464,43 @@ func runShell(dir string, cmds ...string) error { cmd = exec.Command("sh", "-c", args) } cmd.Dir = dir - if err := cmd.Run(); err != nil { - return errors.Wrapf(err, "error running %v", args) - } + cmd.Stderr = os.Stderr + require.NoErrorf(t, cmd.Run(), "error running %v", args) } - return nil +} + +func logProgressStreams(ctx context.Context, t *testing.T) context.Context { + pr, ctx, cancel := progress.NewContext(ctx) + done := make(chan struct{}) + t.Cleanup(func() { + cancel() + <-done + }) + go func() { + defer close(done) + for { + prog, err := pr.Read(context.Background()) + if err != nil { + return + } + for _, log := range prog { + switch lsys := log.Sys.(type) { + case client.VertexLog: + var stream string + switch lsys.Stream { + case 1: + stream = "stdout" + case 2: + stream = "stderr" + default: + stream = strconv.FormatInt(int64(lsys.Stream), 10) + } + t.Logf("(%v) %s", stream, lsys.Data) + default: + t.Logf("(%T) %+v", log.Sys, log) + } + } + } + }() + return ctx }