From d03e93f6f1d45340c2a7bb259cb398a88b4c0945 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Thu, 25 May 2023 13:54:26 +0100 Subject: [PATCH 1/8] test: tmpdir can be a test helper Signed-off-by: Justin Chadwell --- tests/build.go | 3 +-- tests/integration.go | 11 ++++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/build.go b/tests/build.go index 71908546..53bc74d2 100644 --- a/tests/build.go +++ b/tests/build.go @@ -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 } diff --git a/tests/integration.go b/tests/integration.go index 0a0276bc..a1a860f0 100644 --- a/tests/integration.go +++ b/tests/integration.go @@ -7,14 +7,15 @@ 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 { - return "", err - } - return tmpdir, nil + err := fstest.Apply(appliers...).Apply(tmpdir) + require.NoError(t, err) + return tmpdir } func buildxCmd(sb integration.Sandbox, args ...string) *exec.Cmd { From 48d7dafbd59b738c7a07a0793f6a0e5236085ffd Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Thu, 25 May 2023 13:55:26 +0100 Subject: [PATCH 2/8] git: update gitutil test utilities - Adds a new GitServeHTTP function to start an http server to serve a target git repository. - Adds a new GitDir helper method to get the path to the .git directory - Updates the GitAdd method to take a variable number of files Signed-off-by: Justin Chadwell --- util/gitutil/gitutil.go | 9 +++++ util/gitutil/testutil.go | 5 +-- util/gitutil/testutilserve.go | 62 +++++++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 util/gitutil/testutilserve.go diff --git a/util/gitutil/gitutil.go b/util/gitutil/gitutil.go index b933cb46..8ab76ab2 100644 --- a/util/gitutil/gitutil.go +++ b/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 != "" { diff --git a/util/gitutil/testutil.go b/util/gitutil/testutil.go index 8cdabc15..ea9dd058 100644 --- a/util/gitutil/testutil.go +++ b/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) } diff --git a/util/gitutil/testutilserve.go b/util/gitutil/testutilserve.go new file mode 100644 index 00000000..631ed693 --- /dev/null +++ b/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) +} From 76c96347ffe4831948418b0b84db77d7f1777275 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Thu, 25 May 2023 13:58:10 +0100 Subject: [PATCH 3/8] tests: add basic remote bake context test Signed-off-by: Justin Chadwell --- tests/bake.go | 53 +++++++++++++++++++++++++++++++++++++++ tests/integration_test.go | 1 + 2 files changed, 54 insertions(+) create mode 100644 tests/bake.go diff --git a/tests/bake.go b/tests/bake.go new file mode 100644 index 00000000..0e64d1d1 --- /dev/null +++ b/tests/bake.go @@ -0,0 +1,53 @@ +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, dir string, args ...string) (string, error) { + args = append([]string{"bake", "--progress=quiet"}, args...) + cmd := buildxCmd(sb, args...) + cmd.Dir = dir + out, err := cmd.CombinedOutput() + return string(out), err +} + +var bakeTests = []func(t *testing.T, sb integration.Sandbox){ + testBakeRemote, +} + +func testBakeRemote(t *testing.T, sb integration.Sandbox) { + bakefile := []byte(` +target "default" { + dockerfile-inline = < Date: Thu, 25 May 2023 13:58:52 +0100 Subject: [PATCH 4/8] bake: fix BAKE_CMD_CONTEXT relative path resolution Signed-off-by: Justin Chadwell --- bake/bake.go | 4 ++++ tests/bake.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/bake/bake.go b/bake/bake.go index b3427c07..8dd3b979 100644 --- a/bake/bake.go +++ b/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 diff --git a/tests/bake.go b/tests/bake.go index 0e64d1d1..7e17aa53 100644 --- a/tests/bake.go +++ b/tests/bake.go @@ -20,6 +20,7 @@ func bakeCmd(sb integration.Sandbox, dir string, args ...string) (string, error) var bakeTests = []func(t *testing.T, sb integration.Sandbox){ testBakeRemote, + testBakeRemoteCmdContext, } func testBakeRemote(t *testing.T, sb integration.Sandbox) { @@ -51,3 +52,37 @@ EOT 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 = < Date: Thu, 25 May 2023 15:59:16 +0100 Subject: [PATCH 5/8] tests: add bake test for remote cmd context override Signed-off-by: Justin Chadwell --- tests/bake.go | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/tests/bake.go b/tests/bake.go index 7e17aa53..facd07e9 100644 --- a/tests/bake.go +++ b/tests/bake.go @@ -21,6 +21,7 @@ func bakeCmd(sb integration.Sandbox, dir string, args ...string) (string, error) var bakeTests = []func(t *testing.T, sb integration.Sandbox){ testBakeRemote, testBakeRemoteCmdContext, + testBakeRemoteCmdContextOverride, } func testBakeRemote(t *testing.T, sb integration.Sandbox) { @@ -86,3 +87,43 @@ EOT 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 = < Date: Thu, 25 May 2023 17:54:17 +0100 Subject: [PATCH 6/8] tests: add bake test for remote subdir context Fixed in 12b6a3ad9aeda4bdf67a6a55d22fc43594125ec4, but now we have regression tests! So we can add a check that we don't break this behavior again. Signed-off-by: Justin Chadwell --- tests/bake.go | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/bake.go b/tests/bake.go index facd07e9..62824a6a 100644 --- a/tests/bake.go +++ b/tests/bake.go @@ -22,6 +22,7 @@ var bakeTests = []func(t *testing.T, sb integration.Sandbox){ testBakeRemote, testBakeRemoteCmdContext, testBakeRemoteCmdContextOverride, + testBakeRemoteContextSubdir, } func testBakeRemote(t *testing.T, sb integration.Sandbox) { @@ -127,3 +128,37 @@ EOT 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, "/tmp", addr, "--set", "*.output=type=local,dest="+dirDest) + require.NoError(t, err, out) + + require.FileExists(t, filepath.Join(dirDest, "super-cool.txt")) +} From c820350b5edbb6bf0c98dd90ed90716294ee7fd5 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Tue, 6 Jun 2023 16:38:46 +0200 Subject: [PATCH 7/8] tests: refactor cmd helpers to allow configuring cwd, etc Signed-off-by: Justin Chadwell --- tests/bake.go | 15 +++++++-------- tests/build.go | 26 +++++++++++++------------- tests/inspect.go | 6 +++--- tests/integration.go | 27 +++++++++++++++++++++++---- tests/ls.go | 6 +++--- 5 files changed, 49 insertions(+), 31 deletions(-) diff --git a/tests/bake.go b/tests/bake.go index 62824a6a..a462be70 100644 --- a/tests/bake.go +++ b/tests/bake.go @@ -10,10 +10,9 @@ import ( "github.com/stretchr/testify/require" ) -func bakeCmd(sb integration.Sandbox, dir string, args ...string) (string, error) { - args = append([]string{"bake", "--progress=quiet"}, args...) - cmd := buildxCmd(sb, args...) - cmd.Dir = dir +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 } @@ -49,7 +48,7 @@ EOT gitutil.GitCommit(git, t, "initial commit") addr := gitutil.GitServeHTTP(git, t) - out, err := bakeCmd(sb, dir, addr, "--set", "*.output=type=local,dest="+dirDest) + 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")) @@ -83,7 +82,7 @@ EOT gitutil.GitCommit(git, t, "initial commit") addr := gitutil.GitServeHTTP(git, t) - out, err := bakeCmd(sb, dirSrc, addr, "--set", "*.output=type=local,dest="+dirDest) + 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")) @@ -123,7 +122,7 @@ EOT gitutil.GitCommit(gitSrc, t, "initial commit") addrSrc := gitutil.GitServeHTTP(gitSrc, t) - out, err := bakeCmd(sb, "/tmp", addrSpec, addrSrc, "--set", "*.output=type=local,dest="+dirDest) + 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")) @@ -157,7 +156,7 @@ COPY super-cool.txt / gitutil.GitCommit(git, t, "initial commit") addr := gitutil.GitServeHTTP(git, t) - out, err := bakeCmd(sb, "/tmp", addr, "--set", "*.output=type=local,dest="+dirDest) + 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")) diff --git a/tests/build.go b/tests/build.go index 53bc74d2..f82c80b3 100644 --- a/tests/build.go +++ b/tests/build.go @@ -19,9 +19,9 @@ import ( "github.com/stretchr/testify/require" ) -func buildCmd(sb integration.Sandbox, args ...string) (string, error) { - args = append([]string{"build", "--progress=quiet"}, args...) - cmd := buildxCmd(sb, args...) +func buildCmd(sb integration.Sandbox, opts ...cmdOpt) (string, error) { + opts = append([]cmdOpt{withArgs("build", "--progress=quiet")}, opts...) + 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")) diff --git a/tests/inspect.go b/tests/inspect.go index ec83af13..207d495e 100644 --- a/tests/inspect.go +++ b/tests/inspect.go @@ -8,9 +8,9 @@ import ( "github.com/stretchr/testify/require" ) -func inspectCmd(sb integration.Sandbox, args ...string) (string, error) { - args = append([]string{"inspect"}, args...) - cmd := buildxCmd(sb, args...) +func inspectCmd(sb integration.Sandbox, opts ...cmdOpt) (string, error) { + opts = append([]cmdOpt{withArgs("inspect")}, opts...) + cmd := buildxCmd(sb, opts...) out, err := cmd.CombinedOutput() return string(out), err } diff --git a/tests/integration.go b/tests/integration.go index a1a860f0..9582bc50 100644 --- a/tests/integration.go +++ b/tests/integration.go @@ -18,13 +18,32 @@ func tmpdir(t *testing.T, appliers ...fstest.Applier) string { return tmpdir } -func buildxCmd(sb integration.Sandbox, args ...string) *exec.Cmd { +type cmdOpt func(*exec.Cmd) + +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 != "" { - args = append([]string{"--builder=" + builder}, args...) + 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 diff --git a/tests/ls.go b/tests/ls.go index 87863095..a072d2af 100644 --- a/tests/ls.go +++ b/tests/ls.go @@ -8,9 +8,9 @@ import ( "github.com/stretchr/testify/require" ) -func lsCmd(sb integration.Sandbox, args ...string) (string, error) { - args = append([]string{"ls"}, args...) - cmd := buildxCmd(sb, args...) +func lsCmd(sb integration.Sandbox, opts ...cmdOpt) (string, error) { + opts = append([]cmdOpt{withArgs("ls")}, opts...) + cmd := buildxCmd(sb, opts...) out, err := cmd.CombinedOutput() return string(out), err } From d34103b0d967e8f6da448d1dae42d95ce75c80ac Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Tue, 6 Jun 2023 16:40:18 +0200 Subject: [PATCH 8/8] bake: fix potential context entitlements escape Signed-off-by: Justin Chadwell --- bake/bake.go | 3 +- tests/bake.go | 102 +++++++++++++++++++++++++++++++++++++++++++ tests/integration.go | 6 +++ 3 files changed, 110 insertions(+), 1 deletion(-) diff --git a/bake/bake.go b/bake/bake.go index 8dd3b979..9a507cd6 100644 --- a/bake/bake.go +++ b/bake/bake.go @@ -1012,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 diff --git a/tests/bake.go b/tests/bake.go index a462be70..130fb0bd 100644 --- a/tests/bake.go +++ b/tests/bake.go @@ -22,6 +22,8 @@ var bakeTests = []func(t *testing.T, sb integration.Sandbox){ testBakeRemoteCmdContext, testBakeRemoteCmdContextOverride, testBakeRemoteContextSubdir, + testBakeRemoteCmdContextEscapeRoot, + testBakeRemoteCmdContextEscapeRelative, } func testBakeRemote(t *testing.T, sb integration.Sandbox) { @@ -161,3 +163,103 @@ COPY super-cool.txt / 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 = <