Skip to content

Commit

Permalink
source/git: isolate git from local system
Browse files Browse the repository at this point in the history
Prevent git commands we run from reading the user or system
configuration, or cloning submodules from the local filesystem.

Signed-off-by: Cory Snider <[email protected]>
(cherry picked from commit 48aecb8ee307080f528bd59e84c4873a92670478)
Signed-off-by: Tonis Tiigi <[email protected]>
  • Loading branch information
corhere authored and tonistiigi committed Oct 17, 2022
1 parent 450155e commit c014937
Show file tree
Hide file tree
Showing 2 changed files with 141 additions and 45 deletions.
16 changes: 13 additions & 3 deletions source/git/gitsource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
170 changes: 128 additions & 42 deletions source/git/gitsource_test.go
Original file line number Diff line number Diff line change
@@ -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"

Expand All @@ -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"
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -170,19 +178,18 @@ 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)

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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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",
Expand All @@ -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" {
Expand All @@ -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
}

0 comments on commit c014937

Please sign in to comment.