beenull/moby
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

builder: make git config isolation opt-in

Browse source 
While it is undesirable for the system or user git config to be used
when the daemon clones a Git repo, it could break workflows if it was
unconditionally applied to docker/cli as well.

Signed-off-by: Cory Snider <csnider@mirantis.com>
This commit is contained in:
Cory Snider 2022-10-13 17:28:02 -04:00
parent ca99cab891
commit 20ff8a2380
3 changed files with 61 additions and 40 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
builder/remotecontext/git.go
View file

@ -11,7 +11,7 @@ import (
// MakeGitContext returns a Context from gitURL that is cloned in a temporary directory. // MakeGitContext returns a Context from gitURL that is cloned in a temporary directory.
func MakeGitContext(gitURL string) (builder.Source, error) { func MakeGitContext(gitURL string) (builder.Source, error) {
root, err := git.Clone(gitURL) root, err := git.Clone(gitURL, git.WithIsolatedConfig(true))
if err != nil { if err != nil {
return nil, err return nil, err
} }

53
builder/remotecontext/git/gitutils.go
View file

@ -16,21 +16,37 @@ type gitRepo struct {
remote string remote string
ref string ref string
subdir string subdir string
isolateConfig bool
}
type CloneOption func(*gitRepo)
// WithIsolatedConfig disables reading the user or system gitconfig files when
// performing Git operations.
func WithIsolatedConfig(v bool) CloneOption {
return func(gr *gitRepo) {
gr.isolateConfig = v
}
} }
// Clone clones a repository into a newly created directory which // Clone clones a repository into a newly created directory which
// will be under "docker-build-git" // will be under "docker-build-git"
func Clone(remoteURL string) (string, error) { func Clone(remoteURL string, opts ...CloneOption) (string, error) {
repo, err := parseRemoteURL(remoteURL) repo, err := parseRemoteURL(remoteURL)
if err != nil { if err != nil {
return "", err return "", err
} }
return cloneGitRepo(repo) for _, opt := range opts {
opt(&repo)
}
return repo.clone()
} }
func cloneGitRepo(repo gitRepo) (checkoutDir string, err error) { func (repo gitRepo) clone() (checkoutDir string, err error) {
fetch := fetchArgs(repo.remote, repo.ref) fetch := fetchArgs(repo.remote, repo.ref)
root, err := os.MkdirTemp("", "docker-build-git") root, err := os.MkdirTemp("", "docker-build-git")
@ -44,21 +60,21 @@ func cloneGitRepo(repo gitRepo) (checkoutDir string, err error) {
} }
}() }()
if out, err := gitWithinDir(root, "init"); err != nil { if out, err := repo.gitWithinDir(root, "init"); err != nil {
return "", errors.Wrapf(err, "failed to init repo at %s: %s", root, out) return "", errors.Wrapf(err, "failed to init repo at %s: %s", root, out)
} }
// Add origin remote for compatibility with previous implementation that // Add origin remote for compatibility with previous implementation that
// used "git clone" and also to make sure local refs are created for branches // used "git clone" and also to make sure local refs are created for branches
if out, err := gitWithinDir(root, "remote", "add", "origin", repo.remote); err != nil { if out, err := repo.gitWithinDir(root, "remote", "add", "origin", repo.remote); err != nil {
return "", errors.Wrapf(err, "failed add origin repo at %s: %s", repo.remote, out) return "", errors.Wrapf(err, "failed add origin repo at %s: %s", repo.remote, out)
} }
if output, err := gitWithinDir(root, fetch...); err != nil { if output, err := repo.gitWithinDir(root, fetch...); err != nil {
return "", errors.Wrapf(err, "error fetching: %s", output) return "", errors.Wrapf(err, "error fetching: %s", output)
} }
checkoutDir, err = checkoutGit(root, repo.ref, repo.subdir) checkoutDir, err = repo.checkout(root)
if err != nil { if err != nil {
return "", err return "", err
} }
@ -162,20 +178,20 @@ func supportsShallowClone(remoteURL string) bool {
return true return true
} }
func checkoutGit(root, ref, subdir string) (string, error) { func (repo gitRepo) checkout(root string) (string, error) {
// Try checking out by ref name first. This will work on branches and sets // Try checking out by ref name first. This will work on branches and sets
// .git/HEAD to the current branch name // .git/HEAD to the current branch name
if output, err := gitWithinDir(root, "checkout", ref); err != nil { if output, err := repo.gitWithinDir(root, "checkout", repo.ref); err != nil {
// If checking out by branch name fails check out the last fetched ref // If checking out by branch name fails check out the last fetched ref
if _, err2 := gitWithinDir(root, "checkout", "FETCH_HEAD"); err2 != nil { if _, err2 := repo.gitWithinDir(root, "checkout", "FETCH_HEAD"); err2 != nil {
return "", errors.Wrapf(err, "error checking out %s: %s", ref, output) return "", errors.Wrapf(err, "error checking out %s: %s", repo.ref, output)
} }
} }
if subdir != "" { if repo.subdir != "" {
newCtx, err := symlink.FollowSymlinkInScope(filepath.Join(root, subdir), root) newCtx, err := symlink.FollowSymlinkInScope(filepath.Join(root, repo.subdir), root)
if err != nil { if err != nil {
return "", errors.Wrapf(err, "error setting git context, %q not within git root", subdir) return "", errors.Wrapf(err, "error setting git context, %q not within git root", repo.subdir)
} }
fi, err := os.Stat(newCtx) fi, err := os.Stat(newCtx)
@ -191,15 +207,20 @@ func checkoutGit(root, ref, subdir string) (string, error) {
return root, nil return root, nil
} }
func gitWithinDir(dir string, args ...string) ([]byte, error) { func (repo gitRepo) gitWithinDir(dir string, args ...string) ([]byte, error) {
args = append([]string{"-c", "protocol.file.allow=never"}, args...) // Block sneaky repositories from using repos from the filesystem as submodules. args = append([]string{"-c", "protocol.file.allow=never"}, args...) // Block sneaky repositories from using repos from the filesystem as submodules.
cmd := exec.Command("git", args...) cmd := exec.Command("git", args...)
cmd.Dir = dir cmd.Dir = dir
// Disable unsafe remote protocols.
cmd.Env = append(cmd.Env, "GIT_PROTOCOL_FROM_USER=0")
if repo.isolateConfig {
cmd.Env = append(cmd.Env, cmd.Env = append(cmd.Env,
"GIT_PROTOCOL_FROM_USER=0", // Disable unsafe remote protocols.
"GIT_CONFIG_NOSYSTEM=1", // Disable reading from system gitconfig. "GIT_CONFIG_NOSYSTEM=1", // Disable reading from system gitconfig.
"HOME=/dev/null", // Disable reading from user gitconfig. "HOME=/dev/null", // Disable reading from user gitconfig.
) )
}
return cmd.CombinedOutput() return cmd.CombinedOutput()
} }

38
builder/remotecontext/git/gitutils_test.go
View file

@ -162,7 +162,7 @@ func TestCloneArgsGit(t *testing.T) {
} }
func gitGetConfig(name string) string { func gitGetConfig(name string) string {
b, err := gitWithinDir("", "config", "--get", name) b, err := gitRepo{}.gitWithinDir("", "config", "--get", name)
if err != nil { if err != nil {
// since we are interested in empty or non empty string, // since we are interested in empty or non empty string,
// we can safely ignore the err here. // we can safely ignore the err here.
@ -236,9 +236,9 @@ func TestCheckoutGit(t *testing.T) {
} }
gitDir := filepath.Join(root, "repo") gitDir := filepath.Join(root, "repo")
must(gitWithinDir(root, "-c", "init.defaultBranch=master", "init", gitDir)) must(gitRepo{}.gitWithinDir(root, "-c", "init.defaultBranch=master", "init", gitDir))
must(gitWithinDir(gitDir, "config", "user.email", "test@docker.com")) must(gitRepo{}.gitWithinDir(gitDir, "config", "user.email", "test@docker.com"))
must(gitWithinDir(gitDir, "config", "user.name", "Docker test")) must(gitRepo{}.gitWithinDir(gitDir, "config", "user.name", "Docker test"))
assert.NilError(t, os.WriteFile(filepath.Join(gitDir, "Dockerfile"), []byte("FROM scratch"), 0644)) assert.NilError(t, os.WriteFile(filepath.Join(gitDir, "Dockerfile"), []byte("FROM scratch"), 0644))
subDir := filepath.Join(gitDir, "subdir") subDir := filepath.Join(gitDir, "subdir")
@ -250,31 +250,31 @@ func TestCheckoutGit(t *testing.T) {
assert.NilError(t, os.Symlink("/subdir", filepath.Join(gitDir, "absolutelink"))) assert.NilError(t, os.Symlink("/subdir", filepath.Join(gitDir, "absolutelink")))
} }
must(gitWithinDir(gitDir, "add", "-A")) must(gitRepo{}.gitWithinDir(gitDir, "add", "-A"))
must(gitWithinDir(gitDir, "commit", "-am", "First commit")) must(gitRepo{}.gitWithinDir(gitDir, "commit", "-am", "First commit"))
must(gitWithinDir(gitDir, "checkout", "-b", "test")) must(gitRepo{}.gitWithinDir(gitDir, "checkout", "-b", "test"))
assert.NilError(t, os.WriteFile(filepath.Join(gitDir, "Dockerfile"), []byte("FROM scratch\nEXPOSE 3000"), 0644)) assert.NilError(t, os.WriteFile(filepath.Join(gitDir, "Dockerfile"), []byte("FROM scratch\nEXPOSE 3000"), 0644))
assert.NilError(t, os.WriteFile(filepath.Join(subDir, "Dockerfile"), []byte("FROM busybox\nEXPOSE 5000"), 0644)) assert.NilError(t, os.WriteFile(filepath.Join(subDir, "Dockerfile"), []byte("FROM busybox\nEXPOSE 5000"), 0644))
must(gitWithinDir(gitDir, "add", "-A")) must(gitRepo{}.gitWithinDir(gitDir, "add", "-A"))
must(gitWithinDir(gitDir, "commit", "-am", "Branch commit")) must(gitRepo{}.gitWithinDir(gitDir, "commit", "-am", "Branch commit"))
must(gitWithinDir(gitDir, "checkout", "master")) must(gitRepo{}.gitWithinDir(gitDir, "checkout", "master"))
// set up submodule // set up submodule
subrepoDir := filepath.Join(root, "subrepo") subrepoDir := filepath.Join(root, "subrepo")
must(gitWithinDir(root, "-c", "init.defaultBranch=master", "init", subrepoDir)) must(gitRepo{}.gitWithinDir(root, "-c", "init.defaultBranch=master", "init", subrepoDir))
must(gitWithinDir(subrepoDir, "config", "user.email", "test@docker.com")) must(gitRepo{}.gitWithinDir(subrepoDir, "config", "user.email", "test@docker.com"))
must(gitWithinDir(subrepoDir, "config", "user.name", "Docker test")) must(gitRepo{}.gitWithinDir(subrepoDir, "config", "user.name", "Docker test"))
assert.NilError(t, os.WriteFile(filepath.Join(subrepoDir, "subfile"), []byte("subcontents"), 0644)) assert.NilError(t, os.WriteFile(filepath.Join(subrepoDir, "subfile"), []byte("subcontents"), 0644))
must(gitWithinDir(subrepoDir, "add", "-A")) must(gitRepo{}.gitWithinDir(subrepoDir, "add", "-A"))
must(gitWithinDir(subrepoDir, "commit", "-am", "Subrepo initial")) must(gitRepo{}.gitWithinDir(subrepoDir, "commit", "-am", "Subrepo initial"))
must(gitWithinDir(gitDir, "submodule", "add", server.URL+"/subrepo", "sub")) must(gitRepo{}.gitWithinDir(gitDir, "submodule", "add", server.URL+"/subrepo", "sub"))
must(gitWithinDir(gitDir, "add", "-A")) must(gitRepo{}.gitWithinDir(gitDir, "add", "-A"))
must(gitWithinDir(gitDir, "commit", "-am", "With submodule")) must(gitRepo{}.gitWithinDir(gitDir, "commit", "-am", "With submodule"))
type singleCase struct { type singleCase struct {
frag string frag string
@ -311,7 +311,7 @@ func TestCheckoutGit(t *testing.T) {
t.Run(c.frag, func(t *testing.T) { t.Run(c.frag, func(t *testing.T) {
currentSubtest = t currentSubtest = t
ref, subdir := getRefAndSubdir(c.frag) ref, subdir := getRefAndSubdir(c.frag)
r, err := cloneGitRepo(gitRepo{remote: server.URL + "/repo", ref: ref, subdir: subdir}) r, err := gitRepo{remote: server.URL + "/repo", ref: ref, subdir: subdir}.clone()
if c.fail { if c.fail {
assert.Check(t, is.ErrorContains(err, "")) assert.Check(t, is.ErrorContains(err, ""))