Merge pull request #1840 from jedevc/fix-check-path-for-bake-cmd-context

bake/bake.go

@@ -1000,6 +1000,10 @@ func checkPath(p string) error {
 		}
 		return err
 	}
+	p, err = filepath.Abs(p)
+	if err != nil {
+		return err
+	}
 	wd, err := os.Getwd()
 	if err != nil {
 		return err
@@ -1008,7 +1012,8 @@ func checkPath(p string) error {
 	if err != nil {
 		return err
 	}
-	if strings.HasPrefix(rel, ".."+string(os.PathSeparator)) {
+	parts := strings.Split(rel, string(os.PathSeparator))
+	if parts[0] == ".." {
 		return errors.Errorf("path %s is outside of the working directory, please set BAKE_ALLOW_REMOTE_FS_ACCESS=1", p)
 	}
 	return nil

tests/bake.go

@@ -0,0 +1,265 @@
+package tests
+
+import (
+	"path/filepath"
+	"testing"
+
+	"github.com/containerd/continuity/fs/fstest"
+	"github.com/docker/buildx/util/gitutil"
+	"github.com/moby/buildkit/util/testutil/integration"
+	"github.com/stretchr/testify/require"
+)
+
+func bakeCmd(sb integration.Sandbox, opts ...cmdOpt) (string, error) {
+	opts = append([]cmdOpt{withArgs("bake", "--progress=quiet")}, opts...)
+	cmd := buildxCmd(sb, opts...)
+	out, err := cmd.CombinedOutput()
+	return string(out), err
+}
+
+var bakeTests = []func(t *testing.T, sb integration.Sandbox){
+	testBakeRemote,
+	testBakeRemoteCmdContext,
+	testBakeRemoteCmdContextOverride,
+	testBakeRemoteContextSubdir,
+	testBakeRemoteCmdContextEscapeRoot,
+	testBakeRemoteCmdContextEscapeRelative,
+}
+
+func testBakeRemote(t *testing.T, sb integration.Sandbox) {
+	bakefile := []byte(`
+target "default" {
+	dockerfile-inline = <<EOT
+FROM scratch
+COPY foo /foo
+EOT
+}
+`)
+	dir := tmpdir(
+		t,
+		fstest.CreateFile("docker-bake.hcl", bakefile, 0600),
+		fstest.CreateFile("foo", []byte("foo"), 0600),
+	)
+	dirDest := t.TempDir()
+
+	git, err := gitutil.New(gitutil.WithWorkingDir(dir))
+	require.NoError(t, err)
+
+	gitutil.GitInit(git, t)
+	gitutil.GitAdd(git, t, "docker-bake.hcl", "foo")
+	gitutil.GitCommit(git, t, "initial commit")
+	addr := gitutil.GitServeHTTP(git, t)
+
+	out, err := bakeCmd(sb, withDir(dir), withArgs(addr, "--set", "*.output=type=local,dest="+dirDest))
+	require.NoError(t, err, out)
+
+	require.FileExists(t, filepath.Join(dirDest, "foo"))
+}
+
+func testBakeRemoteCmdContext(t *testing.T, sb integration.Sandbox) {
+	bakefile := []byte(`
+target "default" {
+	context = BAKE_CMD_CONTEXT
+	dockerfile-inline = <<EOT
+FROM scratch
+COPY foo /foo
+EOT
+}
+`)
+	dirSpec := tmpdir(
+		t,
+		fstest.CreateFile("docker-bake.hcl", bakefile, 0600),
+	)
+	dirSrc := tmpdir(
+		t,
+		fstest.CreateFile("foo", []byte("foo"), 0600),
+	)
+	dirDest := t.TempDir()
+
+	git, err := gitutil.New(gitutil.WithWorkingDir(dirSpec))
+	require.NoError(t, err)
+
+	gitutil.GitInit(git, t)
+	gitutil.GitAdd(git, t, "docker-bake.hcl")
+	gitutil.GitCommit(git, t, "initial commit")
+	addr := gitutil.GitServeHTTP(git, t)
+
+	out, err := bakeCmd(sb, withDir(dirSrc), withArgs(addr, "--set", "*.output=type=local,dest="+dirDest))
+	require.NoError(t, err, out)
+
+	require.FileExists(t, filepath.Join(dirDest, "foo"))
+}
+
+func testBakeRemoteCmdContextOverride(t *testing.T, sb integration.Sandbox) {
+	bakefile := []byte(`
+target "default" {
+	context = BAKE_CMD_CONTEXT
+	dockerfile-inline = <<EOT
+FROM scratch
+COPY foo /foo
+EOT
+}
+`)
+	dirSpec := tmpdir(
+		t,
+		fstest.CreateFile("docker-bake.hcl", bakefile, 0600),
+	)
+	dirSrc := tmpdir(
+		t,
+		fstest.CreateFile("foo", []byte("foo"), 0600),
+	)
+	dirDest := t.TempDir()
+
+	gitSpec, err := gitutil.New(gitutil.WithWorkingDir(dirSpec))
+	require.NoError(t, err)
+	gitutil.GitInit(gitSpec, t)
+	gitutil.GitAdd(gitSpec, t, "docker-bake.hcl")
+	gitutil.GitCommit(gitSpec, t, "initial commit")
+	addrSpec := gitutil.GitServeHTTP(gitSpec, t)
+
+	gitSrc, err := gitutil.New(gitutil.WithWorkingDir(dirSrc))
+	require.NoError(t, err)
+	gitutil.GitInit(gitSrc, t)
+	gitutil.GitAdd(gitSrc, t, "foo")
+	gitutil.GitCommit(gitSrc, t, "initial commit")
+	addrSrc := gitutil.GitServeHTTP(gitSrc, t)
+
+	out, err := bakeCmd(sb, withDir("/tmp"), withArgs(addrSpec, addrSrc, "--set", "*.output=type=local,dest="+dirDest))
+	require.NoError(t, err, out)
+
+	require.FileExists(t, filepath.Join(dirDest, "foo"))
+}
+
+// https://github.com/docker/buildx/issues/1738
+func testBakeRemoteContextSubdir(t *testing.T, sb integration.Sandbox) {
+	bakefile := []byte(`
+target default {
+	context = "./bar"
+}
+`)
+	dockerfile := []byte(`
+FROM scratch
+COPY super-cool.txt /
+`)
+
+	dir := tmpdir(
+		t,
+		fstest.CreateFile("docker-bake.hcl", bakefile, 0600),
+		fstest.CreateDir("bar", 0700),
+		fstest.CreateFile("bar/Dockerfile", dockerfile, 0600),
+		fstest.CreateFile("bar/super-cool.txt", []byte("super cool"), 0600),
+	)
+	dirDest := t.TempDir()
+
+	git, err := gitutil.New(gitutil.WithWorkingDir(dir))
+	require.NoError(t, err)
+	gitutil.GitInit(git, t)
+	gitutil.GitAdd(git, t, "docker-bake.hcl", "bar")
+	gitutil.GitCommit(git, t, "initial commit")
+	addr := gitutil.GitServeHTTP(git, t)
+
+	out, err := bakeCmd(sb, withDir("/tmp"), withArgs(addr, "--set", "*.output=type=local,dest="+dirDest))
+	require.NoError(t, err, out)
+
+	require.FileExists(t, filepath.Join(dirDest, "super-cool.txt"))
+}
+
+func testBakeRemoteCmdContextEscapeRoot(t *testing.T, sb integration.Sandbox) {
+	dirSrc := tmpdir(
+		t,
+		fstest.CreateFile("foo", []byte("foo"), 0600),
+	)
+	dirSrc, err := filepath.Abs(dirSrc)
+	require.NoError(t, err)
+
+	dirCurrent := tmpdir(t)
+	dirCurrent, err = filepath.Abs(dirCurrent)
+	require.NoError(t, err)
+
+	bakefile := []byte(`
+target "default" {
+	context = "cwd://` + dirSrc + `"
+	dockerfile-inline = <<EOT
+FROM scratch
+COPY foo /foo
+EOT
+}
+`)
+	dirSpec := tmpdir(
+		t,
+		fstest.CreateFile("docker-bake.hcl", bakefile, 0600),
+	)
+	dirDest := t.TempDir()
+
+	git, err := gitutil.New(gitutil.WithWorkingDir(dirSpec))
+	require.NoError(t, err)
+
+	gitutil.GitInit(git, t)
+	gitutil.GitAdd(git, t, "docker-bake.hcl")
+	gitutil.GitCommit(git, t, "initial commit")
+	addr := gitutil.GitServeHTTP(git, t)
+
+	out, err := bakeCmd(
+		sb,
+		withDir(dirCurrent),
+		withArgs(addr, "--set", "*.output=type=local,dest="+dirDest),
+	)
+	require.Error(t, err, out)
+	require.Contains(t, out, "outside of the working directory, please set BAKE_ALLOW_REMOTE_FS_ACCESS")
+
+	out, err = bakeCmd(
+		sb,
+		withDir(dirCurrent),
+		withArgs(addr, "--set", "*.output=type=local,dest="+dirDest),
+		withEnv("BAKE_ALLOW_REMOTE_FS_ACCESS=1"),
+	)
+	require.NoError(t, err, out)
+	require.FileExists(t, filepath.Join(dirDest, "foo"))
+}
+
+func testBakeRemoteCmdContextEscapeRelative(t *testing.T, sb integration.Sandbox) {
+	bakefile := []byte(`
+target "default" {
+	context = "cwd://../"
+	dockerfile-inline = <<EOT
+FROM scratch
+COPY foo /foo
+EOT
+}
+`)
+	dirSpec := tmpdir(
+		t,
+		fstest.CreateFile("docker-bake.hcl", bakefile, 0600),
+	)
+	dirSrc := tmpdir(
+		t,
+		fstest.CreateFile("foo", []byte("foo"), 0600),
+		fstest.CreateDir("subdir", 0700),
+	)
+	dirDest := t.TempDir()
+
+	git, err := gitutil.New(gitutil.WithWorkingDir(dirSpec))
+	require.NoError(t, err)
+
+	gitutil.GitInit(git, t)
+	gitutil.GitAdd(git, t, "docker-bake.hcl")
+	gitutil.GitCommit(git, t, "initial commit")
+	addr := gitutil.GitServeHTTP(git, t)
+
+	out, err := bakeCmd(
+		sb,
+		withDir(filepath.Join(dirSrc, "subdir")),
+		withArgs(addr, "--set", "*.output=type=local,dest="+dirDest),
+	)
+	require.Error(t, err, out)
+	require.Contains(t, out, "outside of the working directory, please set BAKE_ALLOW_REMOTE_FS_ACCESS")
+
+	out, err = bakeCmd(
+		sb,
+		withDir(filepath.Join(dirSrc, "subdir")),
+		withArgs(addr, "--set", "*.output=type=local,dest="+dirDest),
+		withEnv("BAKE_ALLOW_REMOTE_FS_ACCESS=1"),
+	)
+	require.NoError(t, err, out)
+	require.FileExists(t, filepath.Join(dirDest, "foo"))
+}

tests/build.go

@@ -19,9 +19,9 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func buildCmd(sb integration.Sandbox, args ...string) (string, error) {
+func buildCmd(sb integration.Sandbox, opts ...cmdOpt) (string, error) {
-	args = append([]string{"build", "--progress=quiet"}, args...)
+	opts = append([]cmdOpt{withArgs("build", "--progress=quiet")}, opts...)
-	cmd := buildxCmd(sb, args...)
+	cmd := buildxCmd(sb, opts...)
 	out, err := cmd.CombinedOutput()
 	return string(out), err
 }
@@ -36,13 +36,13 @@ var buildTests = []func(t *testing.T, sb integration.Sandbox){
 
 func testBuild(t *testing.T, sb integration.Sandbox) {
 	dir := createTestProject(t)
-	out, err := buildCmd(sb, dir)
+	out, err := buildCmd(sb, withArgs(dir))
 	require.NoError(t, err, string(out))
 }
 
 func testBuildLocalExport(t *testing.T, sb integration.Sandbox) {
 	dir := createTestProject(t)
-	out, err := buildCmd(sb, fmt.Sprintf("--output=type=local,dest=%s/result", dir), dir)
+	out, err := buildCmd(sb, withArgs(fmt.Sprintf("--output=type=local,dest=%s/result", dir), dir))
 	require.NoError(t, err, string(out))
 
 	dt, err := os.ReadFile(dir + "/result/bar")
@@ -52,7 +52,7 @@ func testBuildLocalExport(t *testing.T, sb integration.Sandbox) {
 
 func testBuildTarExport(t *testing.T, sb integration.Sandbox) {
 	dir := createTestProject(t)
-	out, err := buildCmd(sb, fmt.Sprintf("--output=type=tar,dest=%s/result.tar", dir), dir)
+	out, err := buildCmd(sb, withArgs(fmt.Sprintf("--output=type=tar,dest=%s/result.tar", dir), dir))
 	require.NoError(t, err, string(out))
 
 	dt, err := os.ReadFile(fmt.Sprintf("%s/result.tar", dir))
@@ -74,7 +74,7 @@ func testBuildRegistryExport(t *testing.T, sb integration.Sandbox) {
 	require.NoError(t, err)
 	target := registry + "/buildx/registry:latest"
 
-	out, err := buildCmd(sb, fmt.Sprintf("--output=type=image,name=%s,push=true", target), dir)
+	out, err := buildCmd(sb, withArgs(fmt.Sprintf("--output=type=image,name=%s,push=true", target), dir))
 	require.NoError(t, err, string(out))
 
 	desc, provider, err := contentutil.ProviderFromRef(target)
@@ -92,11 +92,9 @@ func testBuildRegistryExport(t *testing.T, sb integration.Sandbox) {
 func testImageIDOutput(t *testing.T, sb integration.Sandbox) {
 	dockerfile := []byte(`FROM busybox:latest`)
 
-	dir, err := tmpdir(t,
+	dir := tmpdir(t,
 		fstest.CreateFile("Dockerfile", dockerfile, 0600),
 	)
-	require.NoError(t, err)
-
 	targetDir := t.TempDir()
 
 	outFlag := "--output=type=docker"
@@ -106,12 +104,14 @@ func testImageIDOutput(t *testing.T, sb integration.Sandbox) {
 		outFlag += ",dest=" + targetDir + "/image.tar"
 	}
 
-	cmd := buildxCmd(sb, "build", "-q", outFlag, "--iidfile", filepath.Join(targetDir, "iid.txt"), "--metadata-file", filepath.Join(targetDir, "md.json"), dir)
+	cmd := buildxCmd(
+		sb,
+		withArgs("build", "-q", outFlag, "--iidfile", filepath.Join(targetDir, "iid.txt"), "--metadata-file", filepath.Join(targetDir, "md.json"), dir),
+	)
 	stdout := bytes.NewBuffer(nil)
 	cmd.Stdout = stdout
 	cmd.Stderr = os.Stderr
-	err = cmd.Run()
+	err := cmd.Run()
-
 	require.NoError(t, err)
 
 	dt, err := os.ReadFile(filepath.Join(targetDir, "iid.txt"))
@@ -148,11 +148,10 @@ RUN cp /etc/foo /etc/bar
 FROM scratch
 COPY --from=base /etc/bar /bar
 `)
-	dir, err := tmpdir(
+	dir := tmpdir(
 		t,
 		fstest.CreateFile("Dockerfile", dockerfile, 0600),
 		fstest.CreateFile("foo", []byte("foo"), 0600),
 	)
-	require.NoError(t, err)
 	return dir
 }

tests/inspect.go

@@ -8,9 +8,9 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func inspectCmd(sb integration.Sandbox, args ...string) (string, error) {
+func inspectCmd(sb integration.Sandbox, opts ...cmdOpt) (string, error) {
-	args = append([]string{"inspect"}, args...)
+	opts = append([]cmdOpt{withArgs("inspect")}, opts...)
-	cmd := buildxCmd(sb, args...)
+	cmd := buildxCmd(sb, opts...)
 	out, err := cmd.CombinedOutput()
 	return string(out), err
 }

tests/integration.go

@@ -7,23 +7,49 @@ import (
 
 	"github.com/containerd/continuity/fs/fstest"
 	"github.com/moby/buildkit/util/testutil/integration"
+	"github.com/stretchr/testify/require"
 )
 
-func tmpdir(t *testing.T, appliers ...fstest.Applier) (string, error) {
+func tmpdir(t *testing.T, appliers ...fstest.Applier) string {
+	t.Helper()
 	tmpdir := t.TempDir()
-	if err := fstest.Apply(appliers...).Apply(tmpdir); err != nil {
+	err := fstest.Apply(appliers...).Apply(tmpdir)
-		return "", err
+	require.NoError(t, err)
-	}
+	return tmpdir
-	return tmpdir, nil
 }
 
-func buildxCmd(sb integration.Sandbox, args ...string) *exec.Cmd {
+type cmdOpt func(*exec.Cmd)
-	if builder := sb.Address(); builder != "" {
+
-		args = append([]string{"--builder=" + builder}, args...)
+func withEnv(env ...string) cmdOpt {
+	return func(cmd *exec.Cmd) {
+		cmd.Env = append(cmd.Env, env...)
+	}
+}
+
+func withArgs(args ...string) cmdOpt {
+	return func(cmd *exec.Cmd) {
+		cmd.Args = append(cmd.Args, args...)
+	}
+}
+
+func withDir(dir string) cmdOpt {
+	return func(cmd *exec.Cmd) {
+		cmd.Dir = dir
+	}
+}
+
+func buildxCmd(sb integration.Sandbox, opts ...cmdOpt) *exec.Cmd {
+	cmd := exec.Command("buildx")
+	cmd.Env = append([]string{}, os.Environ()...)
+	for _, opt := range opts {
+		opt(cmd)
+	}
+
+	if builder := sb.Address(); builder != "" {
+		cmd.Args = append(cmd.Args, "--builder="+builder)
 	}
-	cmd := exec.Command("buildx", args...)
 	if context := sb.DockerAddress(); context != "" {
-		cmd.Env = append(os.Environ(), "DOCKER_CONTEXT="+context)
+		cmd.Env = append(cmd.Env, "DOCKER_CONTEXT="+context)
 	}
 
 	return cmd

tests/integration_test.go

@@ -21,6 +21,7 @@ func init() {
 func TestIntegration(t *testing.T) {
 	var tests []func(t *testing.T, sb integration.Sandbox)
 	tests = append(tests, buildTests...)
+	tests = append(tests, bakeTests...)
 	tests = append(tests, inspectTests...)
 	tests = append(tests, lsTests...)
 	testIntegration(t, tests...)

tests/ls.go

@@ -8,9 +8,9 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func lsCmd(sb integration.Sandbox, args ...string) (string, error) {
+func lsCmd(sb integration.Sandbox, opts ...cmdOpt) (string, error) {
-	args = append([]string{"ls"}, args...)
+	opts = append([]cmdOpt{withArgs("ls")}, opts...)
-	cmd := buildxCmd(sb, args...)
+	cmd := buildxCmd(sb, opts...)
 	out, err := cmd.CombinedOutput()
 	return string(out), err
 }

util/gitutil/gitutil.go

@@ -6,6 +6,7 @@ import (
 	"net/url"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"strings"
 
 	"github.com/pkg/errors"
@@ -68,6 +69,14 @@ func (c *Git) RootDir() (string, error) {
 	return c.clean(c.run("rev-parse", "--show-toplevel"))
 }
 
+func (c *Git) GitDir() (string, error) {
+	dir, err := c.RootDir()
+	if err != nil {
+		return "", err
+	}
+	return filepath.Join(dir, ".git"), nil
+}
+
 func (c *Git) RemoteURL() (string, error) {
 	// Try to get the remote URL from the origin remote first
 	if ru, err := c.clean(c.run("remote", "get-url", "origin")); err == nil && ru != "" {

util/gitutil/testutil.go

@@ -39,9 +39,10 @@ func GitCheckoutBranch(c *Git, tb testing.TB, name string) {
 	require.Empty(tb, out)
 }
 
-func GitAdd(c *Git, tb testing.TB, file string) {
+func GitAdd(c *Git, tb testing.TB, files ...string) {
 	tb.Helper()
-	_, err := fakeGit(c, "add", file)
+	args := append([]string{"add"}, files...)
+	_, err := fakeGit(c, args...)
 	require.NoError(tb, err)
 }
 

util/gitutil/testutilserve.go

@@ -0,0 +1,62 @@
+package gitutil
+
+import (
+	"context"
+	"fmt"
+	"net"
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func GitServeHTTP(c *Git, t testing.TB) (url string) {
+	t.Helper()
+	gitUpdateServerInfo(c, t)
+	ctx, cancel := context.WithCancel(context.TODO())
+
+	ready := make(chan struct{})
+	done := make(chan struct{})
+
+	name := "test.git"
+	dir, err := c.GitDir()
+	if err != nil {
+		cancel()
+	}
+
+	var addr string
+	go func() {
+		mux := http.NewServeMux()
+		prefix := fmt.Sprintf("/%s/", name)
+		mux.Handle(prefix, http.StripPrefix(prefix, http.FileServer(http.Dir(dir))))
+		l, err := net.Listen("tcp", "localhost:0")
+		if err != nil {
+			panic(err)
+		}
+
+		addr = l.Addr().String()
+
+		close(ready)
+
+		s := http.Server{Handler: mux} //nolint:gosec // potential attacks are not relevant for tests
+		go s.Serve(l)
+		<-ctx.Done()
+		s.Shutdown(context.TODO())
+		l.Close()
+
+		close(done)
+	}()
+	<-ready
+
+	t.Cleanup(func() {
+		cancel()
+		<-done
+	})
+	return fmt.Sprintf("http://%s/%s", addr, name)
+}
+
+func gitUpdateServerInfo(c *Git, tb testing.TB) {
+	tb.Helper()
+	_, err := fakeGit(c, "update-server-info")
+	require.NoError(tb, err)
+}