Skip to content

Commit c014937

Browse files
corheretonistiigi
corhere
authored and
tonistiigi
committed
source/git: isolate git from local system
Prevent git commands we run from reading the user or system configuration, or cloning submodules from the local filesystem. Signed-off-by: Cory Snider <csnider@mirantis.com> (cherry picked from commit 48aecb8ee307080f528bd59e84c4873a92670478) Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
1 parent 450155e commit c014937

2 files changed

Lines changed: 141 additions & 45 deletions

File tree

‎source/git/gitsource.go‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,11 @@ func (gs *gitSource) mountRemote(ctx context.Context, remote string, auth []stri
129129
}()
130130

131131
if initializeRepo {
132-
if _, err := gitWithinDir(ctx, dir, "", "", "", auth, "init", "--bare"); err != nil {
132+
// Explicitly set the Git config 'init.defaultBranch' to the
133+
// implied default to suppress "hint:" output about not having a
134+
// default initial branch name set which otherwise spams unit
135+
// test logs.
136+
if _, err := gitWithinDir(ctx, dir, "", "", "", auth, "-c", "init.defaultBranch=master", "init", "--bare"); err != nil {
133137
return "", nil, errors.Wrapf(err, "failed to init repo at %s", dir)
134138
}
135139

@@ -485,11 +489,14 @@ func (gs *gitSourceHandler) Snapshot(ctx context.Context, g session.Group) (out
485489
if err := os.MkdirAll(checkoutDir, 0711); err != nil {
486490
return nil, err
487491
}
488-
_, err = gitWithinDir(ctx, checkoutDirGit, "", sock, knownHosts, nil, "init")
492+
_, err = gitWithinDir(ctx, checkoutDirGit, "", sock, knownHosts, nil, "-c", "init.defaultBranch=master", "init")
489493
if err != nil {
490494
return nil, err
491495
}
492-
_, err = gitWithinDir(ctx, checkoutDirGit, "", sock, knownHosts, nil, "remote", "add", "origin", gitDir)
496+
// Defense-in-depth: clone using the file protocol to disable local-clone
497+
// optimizations which can be abused on some versions of Git to copy unintended
498+
// host files into the build context.
499+
_, err = gitWithinDir(ctx, checkoutDirGit, "", sock, knownHosts, nil, "remote", "add", "origin", "file://"+gitDir)
493500
if err != nil {
494501
return nil, err
495502
}
@@ -591,6 +598,7 @@ func git(ctx context.Context, dir, sshAuthSock, knownHosts string, args ...strin
591598
stdout, stderr := logs.NewLogStreams(ctx, false)
592599
defer stdout.Close()
593600
defer stderr.Close()
601+
args = append([]string{"-c", "protocol.file.allow=user"}, args...) // Block sneaky repositories from using repos from the filesystem as submodules.
594602
cmd := exec.Command("git", args...)
595603
cmd.Dir = dir // some commands like submodule require this
596604
buf := bytes.NewBuffer(nil)
@@ -603,6 +611,8 @@ func git(ctx context.Context, dir, sshAuthSock, knownHosts string, args ...strin
603611
"GIT_TERMINAL_PROMPT=0",
604612
"GIT_SSH_COMMAND=" + getGitSSHCommand(knownHosts),
605613
// "GIT_TRACE=1",
614+
"GIT_CONFIG_NOSYSTEM=1", // Disable reading from system gitconfig.
615+
"HOME=/dev/null", // Disable reading from user gitconfig.
606616
}
607617
if sshAuthSock != "" {
608618
cmd.Env = append(cmd.Env, "SSH_AUTH_SOCK="+sshAuthSock)

‎source/git/gitsource_test.go‎

Lines changed: 128 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,17 @@
11
package git
22

33
import (
4+
"bytes"
45
"context"
56
"io/ioutil"
7+
"net/http"
8+
"net/http/cgi"
9+
"net/http/httptest"
610
"os"
711
"os/exec"
812
"path/filepath"
913
"runtime"
14+
"strconv"
1015
"strings"
1116
"testing"
1217

@@ -17,10 +22,12 @@ import (
1722
"github.com/containerd/containerd/snapshots/native"
1823
"github.com/moby/buildkit/cache"
1924
"github.com/moby/buildkit/cache/metadata"
25+
"github.com/moby/buildkit/client"
2026
"github.com/moby/buildkit/snapshot"
2127
containerdsnapshot "github.com/moby/buildkit/snapshot/containerd"
2228
"github.com/moby/buildkit/source"
2329
"github.com/moby/buildkit/util/leaseutil"
30+
"github.com/moby/buildkit/util/progress"
2431
"github.com/pkg/errors"
2532
"github.com/stretchr/testify/assert"
2633
"github.com/stretchr/testify/require"
@@ -40,7 +47,7 @@ func testRepeatedFetch(t *testing.T, keepGitDir bool) {
4047
}
4148

4249
t.Parallel()
43-
ctx := context.TODO()
50+
ctx := logProgressStreams(context.Background(), t)
4451

4552
tmpdir, err := ioutil.TempDir("", "buildkit-state")
4653
require.NoError(t, err)
@@ -52,10 +59,10 @@ func testRepeatedFetch(t *testing.T, keepGitDir bool) {
5259
require.NoError(t, err)
5360
defer os.RemoveAll(repodir)
5461

55-
repodir, err = setupGitRepo(repodir)
62+
repo := setupGitRepo(t, repodir)
5663
require.NoError(t, err)
5764

58-
id := &source.GitIdentifier{Remote: repodir, KeepGitDir: keepGitDir}
65+
id := &source.GitIdentifier{Remote: repo.mainURL, KeepGitDir: keepGitDir}
5966

6067
g, err := gs.Resolve(ctx, id, nil, nil)
6168
require.NoError(t, err)
@@ -97,7 +104,7 @@ func testRepeatedFetch(t *testing.T, keepGitDir bool) {
97104
require.True(t, errors.Is(err, os.ErrNotExist))
98105

99106
// second fetch returns same dir
100-
id = &source.GitIdentifier{Remote: repodir, Ref: "master", KeepGitDir: keepGitDir}
107+
id = &source.GitIdentifier{Remote: repo.mainURL, Ref: "master", KeepGitDir: keepGitDir}
101108

102109
g, err = gs.Resolve(ctx, id, nil, nil)
103110
require.NoError(t, err)
@@ -113,7 +120,7 @@ func testRepeatedFetch(t *testing.T, keepGitDir bool) {
113120

114121
require.Equal(t, ref1.ID(), ref2.ID())
115122

116-
id = &source.GitIdentifier{Remote: repodir, Ref: "feature", KeepGitDir: keepGitDir}
123+
id = &source.GitIdentifier{Remote: repo.mainURL, Ref: "feature", KeepGitDir: keepGitDir}
117124

118125
g, err = gs.Resolve(ctx, id, nil, nil)
119126
require.NoError(t, err)
@@ -159,6 +166,7 @@ func testFetchBySHA(t *testing.T, keepGitDir bool) {
159166

160167
t.Parallel()
161168
ctx := namespaces.WithNamespace(context.Background(), "buildkit-test")
169+
ctx = logProgressStreams(ctx, t)
162170

163171
tmpdir, err := ioutil.TempDir("", "buildkit-state")
164172
require.NoError(t, err)
@@ -170,19 +178,18 @@ func testFetchBySHA(t *testing.T, keepGitDir bool) {
170178
require.NoError(t, err)
171179
defer os.RemoveAll(repodir)
172180

173-
repodir, err = setupGitRepo(repodir)
174-
require.NoError(t, err)
181+
repo := setupGitRepo(t, repodir)
175182

176183
cmd := exec.Command("git", "rev-parse", "feature")
177-
cmd.Dir = repodir
184+
cmd.Dir = repo.mainPath
178185

179186
out, err := cmd.Output()
180187
require.NoError(t, err)
181188

182189
sha := strings.TrimSpace(string(out))
183190
require.Equal(t, 40, len(sha))
184191

185-
id := &source.GitIdentifier{Remote: repodir, Ref: sha, KeepGitDir: keepGitDir}
192+
id := &source.GitIdentifier{Remote: repo.mainURL, Ref: sha, KeepGitDir: keepGitDir}
186193

187194
g, err := gs.Resolve(ctx, id, nil, nil)
188195
require.NoError(t, err)
@@ -236,6 +243,7 @@ func testMultipleRepos(t *testing.T, keepGitDir bool) {
236243

237244
t.Parallel()
238245
ctx := namespaces.WithNamespace(context.Background(), "buildkit-test")
246+
ctx = logProgressStreams(ctx, t)
239247

240248
tmpdir, err := ioutil.TempDir("", "buildkit-state")
241249
require.NoError(t, err)
@@ -247,25 +255,24 @@ func testMultipleRepos(t *testing.T, keepGitDir bool) {
247255
require.NoError(t, err)
248256
defer os.RemoveAll(repodir)
249257

250-
repodir, err = setupGitRepo(repodir)
251-
require.NoError(t, err)
258+
repo := setupGitRepo(t, repodir)
252259

253260
repodir2, err := ioutil.TempDir("", "buildkit-gitsource")
254261
require.NoError(t, err)
255262
defer os.RemoveAll(repodir2)
256263

257-
err = runShell(repodir2,
258-
"git init",
264+
runShell(t, repodir2,
265+
"git -c init.defaultBranch=master init",
259266
"git config --local user.email test",
260267
"git config --local user.name test",
261268
"echo xyz > xyz",
262269
"git add xyz",
263270
"git commit -m initial",
264271
)
265-
require.NoError(t, err)
272+
repoURL2 := serveGitRepo(t, repodir2)
266273

267-
id := &source.GitIdentifier{Remote: repodir, KeepGitDir: keepGitDir}
268-
id2 := &source.GitIdentifier{Remote: repodir2, KeepGitDir: keepGitDir}
274+
id := &source.GitIdentifier{Remote: repo.mainURL, KeepGitDir: keepGitDir}
275+
id2 := &source.GitIdentifier{Remote: repoURL2, KeepGitDir: keepGitDir}
269276

270277
g, err := gs.Resolve(ctx, id, nil, nil)
271278
require.NoError(t, err)
@@ -358,30 +365,33 @@ func setupGitSource(t *testing.T, tmpdir string) source.Source {
358365
return gs
359366
}
360367

361-
func setupGitRepo(dir string) (string, error) {
362-
subPath := filepath.Join(dir, "sub")
363-
mainPath := filepath.Join(dir, "main")
364-
365-
if err := os.MkdirAll(subPath, 0700); err != nil {
366-
return "", err
367-
}
368+
type gitRepoFixture struct {
369+
mainPath, subPath string // Filesystem paths to the respective repos
370+
mainURL, subURL string // HTTP URLs for the respective repos
371+
}
368372

369-
if err := os.MkdirAll(mainPath, 0700); err != nil {
370-
return "", err
373+
func setupGitRepo(t *testing.T, dir string) gitRepoFixture {
374+
t.Helper()
375+
srv := serveGitRepo(t, dir)
376+
fixture := gitRepoFixture{
377+
subPath: filepath.Join(dir, "sub"),
378+
subURL: srv + "/sub",
379+
mainPath: filepath.Join(dir, "main"),
380+
mainURL: srv + "/main",
371381
}
382+
require.NoError(t, os.MkdirAll(fixture.subPath, 0700))
383+
require.NoError(t, os.MkdirAll(fixture.mainPath, 0700))
372384

373-
if err := runShell(filepath.Join(dir, "sub"),
374-
"git init",
385+
runShell(t, fixture.subPath,
386+
"git -c init.defaultBranch=master init",
375387
"git config --local user.email test",
376388
"git config --local user.name test",
377389
"echo subcontents > subfile",
378390
"git add subfile",
379391
"git commit -m initial",
380-
); err != nil {
381-
return "", err
382-
}
383-
if err := runShell(filepath.Join(dir, "main"),
384-
"git init",
392+
)
393+
runShell(t, fixture.mainPath,
394+
"git -c init.defaultBranch=master init",
385395
"git config --local user.email test",
386396
"git config --local user.name test",
387397
"echo foo > abc",
@@ -394,16 +404,58 @@ func setupGitRepo(dir string) (string, error) {
394404
"echo baz > ghi",
395405
"git add ghi",
396406
"git commit -m feature",
397-
"git submodule add "+subPath+" sub",
407+
"git submodule add "+fixture.subURL+" sub",
398408
"git add -A",
399409
"git commit -m withsub",
400-
); err != nil {
401-
return "", err
402-
}
403-
return mainPath, nil
410+
"git checkout master",
411+
)
412+
return fixture
404413
}
405414

406-
func runShell(dir string, cmds ...string) error {
415+
func serveGitRepo(t *testing.T, root string) string {
416+
t.Helper()
417+
gitpath, err := exec.LookPath("git")
418+
require.NoError(t, err)
419+
gitversion, _ := exec.Command(gitpath, "version").CombinedOutput()
420+
t.Logf("%s", gitversion) // E.g. "git version 2.30.2"
421+
422+
// Serve all repositories under root using the Smart HTTP protocol so
423+
// they can be cloned as we explicitly disable the file protocol.
424+
// (Another option would be to use `git daemon` and the Git protocol,
425+
// but that listens on a fixed port number which is a recipe for
426+
// disaster in CI. Funnily enough, `git daemon --port=0` works but there
427+
// is no easy way to discover which port got picked!)
428+
429+
githttp := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
430+
var logs bytes.Buffer
431+
(&cgi.Handler{
432+
Path: gitpath,
433+
Args: []string{"http-backend"},
434+
Dir: root,
435+
Env: []string{
436+
"GIT_PROJECT_ROOT=" + root,
437+
"GIT_HTTP_EXPORT_ALL=1",
438+
},
439+
Stderr: &logs,
440+
}).ServeHTTP(w, r)
441+
if logs.Len() == 0 {
442+
return
443+
}
444+
for {
445+
line, err := logs.ReadString('\n')
446+
t.Log("git-http-backend: " + line)
447+
if err != nil {
448+
break
449+
}
450+
}
451+
})
452+
server := httptest.NewServer(&githttp)
453+
t.Cleanup(server.Close)
454+
return server.URL
455+
}
456+
457+
func runShell(t *testing.T, dir string, cmds ...string) {
458+
t.Helper()
407459
for _, args := range cmds {
408460
var cmd *exec.Cmd
409461
if runtime.GOOS == "windows" {
@@ -412,9 +464,43 @@ func runShell(dir string, cmds ...string) error {
412464
cmd = exec.Command("sh", "-c", args)
413465
}
414466
cmd.Dir = dir
415-
if err := cmd.Run(); err != nil {
416-
return errors.Wrapf(err, "error running %v", args)
417-
}
467+
cmd.Stderr = os.Stderr
468+
require.NoErrorf(t, cmd.Run(), "error running %v", args)
418469
}
419-
return nil
470+
}
471+
472+
func logProgressStreams(ctx context.Context, t *testing.T) context.Context {
473+
pr, ctx, cancel := progress.NewContext(ctx)
474+
done := make(chan struct{})
475+
t.Cleanup(func() {
476+
cancel()
477+
<-done
478+
})
479+
go func() {
480+
defer close(done)
481+
for {
482+
prog, err := pr.Read(context.Background())
483+
if err != nil {
484+
return
485+
}
486+
for _, log := range prog {
487+
switch lsys := log.Sys.(type) {
488+
case client.VertexLog:
489+
var stream string
490+
switch lsys.Stream {
491+
case 1:
492+
stream = "stdout"
493+
case 2:
494+
stream = "stderr"
495+
default:
496+
stream = strconv.FormatInt(int64(lsys.Stream), 10)
497+
}
498+
t.Logf("(%v) %s", stream, lsys.Data)
499+
default:
500+
t.Logf("(%T) %+v", log.Sys, log)
501+
}
502+
}
503+
}
504+
}()
505+
return ctx
420506
}

0 commit comments

Comments
 (0)