Merge pull request #2867 from crazy-max/v0.19_backport_entitlement-path-fix

[v0.19 backport] bake: change evaluation of entitlement paths

Dockerfile

@@ -1,7 +1,7 @@
 # syntax=docker/dockerfile:1
 
 ARG GO_VERSION=1.23
-ARG XX_VERSION=1.5.0
+ARG XX_VERSION=1.6.1
 
 # for testing
 ARG DOCKER_VERSION=27.4.0-rc.2

bake/bake_test.go

@@ -2028,3 +2028,20 @@ target "app" {
 	_, _, err := ReadTargets(ctx, []File{fp}, []string{"app"}, nil, nil, &EntitlementConf{})
 	require.NoError(t, err)
 }
+
+// https://github.com/docker/buildx/issues/2858
+func TestOverrideEmpty(t *testing.T) {
+	fp := File{
+		Name: "docker-bake.hcl",
+		Data: []byte(`
+target "app" {
+  output = ["./bin"]
+}
+`),
+	}
+
+	ctx := context.TODO()
+
+	_, _, err := ReadTargets(ctx, []File{fp}, []string{"app"}, []string{"app.output="}, nil, &EntitlementConf{})
+	require.NoError(t, err)
+}

bake/entitlements.go

@@ -19,6 +19,7 @@ import (
 	"github.com/docker/buildx/util/osutil"
 	"github.com/moby/buildkit/util/entitlements"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 
 type EntitlementKey string
@@ -444,12 +445,20 @@ func evaluatePaths(in []string) ([]string, bool, error) {
 		}
 		v, err := filepath.Abs(p)
 		if err != nil {
-			return nil, false, errors.Wrapf(err, "failed to evaluate path %q", p)
+			logrus.Warnf("failed to evaluate entitlement path %q: %v", p, err)
+			continue
 		}
-		v, err = filepath.EvalSymlinks(v)
+		v, rest, err := evaluateToExistingPath(v)
 		if err != nil {
 			return nil, false, errors.Wrapf(err, "failed to evaluate path %q", p)
 		}
+		v, err = osutil.GetLongPathName(v)
+		if err != nil {
+			return nil, false, errors.Wrapf(err, "failed to evaluate path %q", p)
+		}
+		if rest != "" {
+			v = filepath.Join(v, rest)
+		}
 		out = append(out, v)
 	}
 	return out, allowAny, nil
@@ -458,7 +467,7 @@ func evaluatePaths(in []string) ([]string, bool, error) {
 func evaluateToExistingPaths(in map[string]struct{}) (map[string]struct{}, error) {
 	m := make(map[string]struct{}, len(in))
 	for p := range in {
-		v, err := evaluateToExistingPath(p)
+		v, _, err := evaluateToExistingPath(p)
 		if err != nil {
 			return nil, errors.Wrapf(err, "failed to evaluate path %q", p)
 		}
@@ -471,10 +480,10 @@ func evaluateToExistingPaths(in map[string]struct{}) (map[string]struct{}, error
 	return m, nil
 }
 
-func evaluateToExistingPath(in string) (string, error) {
+func evaluateToExistingPath(in string) (string, string, error) {
 	in, err := filepath.Abs(in)
 	if err != nil {
-		return "", err
+		return "", "", err
 	}
 
 	volLen := volumeNameLen(in)
@@ -529,29 +538,29 @@ func evaluateToExistingPath(in string) (string, error) {
 			if os.IsNotExist(err) {
 				for r := len(dest) - 1; r >= volLen; r-- {
 					if os.IsPathSeparator(dest[r]) {
-						return dest[:r], nil
+						return dest[:r], in[start:], nil
 					}
 				}
-				return vol, nil
+				return vol, in[start:], nil
 			}
-			return "", err
+			return "", "", err
 		}
 
 		if fi.Mode()&fs.ModeSymlink == 0 {
 			if !fi.Mode().IsDir() && end < len(in) {
-				return "", syscall.ENOTDIR
+				return "", "", syscall.ENOTDIR
 			}
 			continue
 		}
 
 		linksWalked++
 		if linksWalked > 255 {
-			return "", errors.New("too many symlinks")
+			return "", "", errors.New("too many symlinks")
 		}
 
 		link, err := os.Readlink(dest)
 		if err != nil {
-			return "", err
+			return "", "", err
 		}
 
 		in = link + in[end:]
@@ -584,7 +593,7 @@ func evaluateToExistingPath(in string) (string, error) {
 			end = 0
 		}
 	}
-	return filepath.Clean(dest), nil
+	return filepath.Clean(dest), "", nil
 }
 
 func volumeNameLen(s string) int {

bake/entitlements_test.go

@@ -89,7 +89,7 @@ func TestEvaluateToExistingPath(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			result, err := evaluateToExistingPath(tt.input)
+			result, _, err := evaluateToExistingPath(tt.input)
 
 			if tt.expectErr {
 				require.Error(t, err)
@@ -341,7 +341,7 @@ func TestValidateEntitlements(t *testing.T) {
 						return nil
 					}
 					// if not, then escapeLink is not allowed
-					exp, err := evaluateToExistingPath(escapeLink)
+					exp, _, err := evaluateToExistingPath(escapeLink)
 					require.NoError(t, err)
 					exp, err = filepath.EvalSymlinks(exp)
 					require.NoError(t, err)
@@ -363,6 +363,48 @@ func TestValidateEntitlements(t *testing.T) {
 			},
 			expected: EntitlementConf{},
 		},
+		{
+			name: "NonExistingAllowedPathSubpath",
+			opt: build.Options{
+				ExportsLocalPathsTemporary: []string{
+					dir1,
+				},
+			},
+			conf: EntitlementConf{
+				FSRead:  []string{wd},
+				FSWrite: []string{filepath.Join(dir1, "not/exists")},
+			},
+			expected: EntitlementConf{
+				FSWrite: []string{expDir1}, // dir1 is still needed as only subpath was allowed
+			},
+		},
+		{
+			name: "NonExistingAllowedPathMatches",
+			opt: build.Options{
+				ExportsLocalPathsTemporary: []string{
+					filepath.Join(dir1, "not/exists"),
+				},
+			},
+			conf: EntitlementConf{
+				FSRead:  []string{wd},
+				FSWrite: []string{filepath.Join(dir1, "not/exists")},
+			},
+			expected: EntitlementConf{
+				FSWrite: []string{expDir1}, // dir1 is still needed as build also needs to write not/exists directory
+			},
+		},
+		{
+			name: "NonExistingBuildPath",
+			opt: build.Options{
+				ExportsLocalPathsTemporary: []string{
+					filepath.Join(dir1, "not/exists"),
+				},
+			},
+			conf: EntitlementConf{
+				FSRead:  []string{wd},
+				FSWrite: []string{dir1},
+			},
+		},
 	}
 
 	for _, tc := range tcases {

tests/bake.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"testing"
 
@@ -46,6 +47,14 @@ var bakeTests = []func(t *testing.T, sb integration.Sandbox){
 	testBakeRemoteDockerfileCwd,
 	testBakeRemoteLocalContextRemoteDockerfile,
 	testBakeEmpty,
+	testBakeSetNonExistingSubdirNoParallel,
+	testBakeSetNonExistingOutsideNoParallel,
+	testBakeSetExistingOutsideNoParallel,
+	testBakeDefinitionNotExistingSubdirNoParallel,
+	testBakeDefinitionNotExistingOutsideNoParallel,
+	testBakeDefinitionExistingOutsideNoParallel,
+	testBakeDefinitionSymlinkOutsideNoParallel,
+	testBakeDefinitionSymlinkOutsideGrantedNoParallel,
 	testBakeShmSize,
 	testBakeUlimits,
 	testBakeMetadataProvenance,
@@ -705,6 +714,261 @@ target "default" {
 	require.Contains(t, string(dt), `size=131072k`)
 }
 
+func testBakeSetNonExistingSubdirNoParallel(t *testing.T, sb integration.Sandbox) {
+	for _, ent := range []bool{true, false} {
+		t.Run(fmt.Sprintf("ent=%v", ent), func(t *testing.T) {
+			t.Setenv("BUILDX_BAKE_ENTITLEMENTS_FS", strconv.FormatBool(ent))
+			dockerfile := []byte(`
+FROM scratch
+COPY foo /foo
+	`)
+			bakefile := []byte(`
+target "default" {
+}
+`)
+			dir := tmpdir(
+				t,
+				fstest.CreateFile("docker-bake.hcl", bakefile, 0600),
+				fstest.CreateFile("Dockerfile", dockerfile, 0600),
+				fstest.CreateFile("foo", []byte("foo"), 0600),
+			)
+
+			cmd := buildxCmd(sb, withDir(dir), withArgs("bake", "--progress=plain", "--set", "*.output=type=local,dest="+filepath.Join(dir, "not/exists")))
+			out, err := cmd.CombinedOutput()
+			require.NoError(t, err, string(out))
+			require.Contains(t, string(out), `#1 [internal] load local bake definitions`)
+			require.Contains(t, string(out), `#1 reading docker-bake.hcl`)
+
+			require.FileExists(t, filepath.Join(dir, "not/exists/foo"))
+		})
+	}
+}
+func testBakeSetNonExistingOutsideNoParallel(t *testing.T, sb integration.Sandbox) {
+	for _, ent := range []bool{true, false} {
+		t.Run(fmt.Sprintf("ent=%v", ent), func(t *testing.T) {
+			t.Setenv("BUILDX_BAKE_ENTITLEMENTS_FS", strconv.FormatBool(ent))
+			dockerfile := []byte(`
+FROM scratch
+COPY foo /foo
+	`)
+			bakefile := []byte(`
+target "default" {
+}
+`)
+			dir := tmpdir(
+				t,
+				fstest.CreateFile("docker-bake.hcl", bakefile, 0600),
+				fstest.CreateFile("Dockerfile", dockerfile, 0600),
+				fstest.CreateFile("foo", []byte("foo"), 0600),
+			)
+
+			destDir := t.TempDir()
+
+			cmd := buildxCmd(sb, withDir(dir), withArgs("bake", "--progress=plain", "--set", "*.output=type=local,dest="+filepath.Join(destDir, "not/exists")))
+			out, err := cmd.CombinedOutput()
+			if ent {
+				require.Error(t, err, string(out))
+				require.Contains(t, string(out), "ERROR: additional privileges requested")
+			} else {
+				require.NoError(t, err, string(out))
+				require.FileExists(t, filepath.Join(destDir, "not/exists/foo"))
+			}
+		})
+	}
+}
+
+func testBakeSetExistingOutsideNoParallel(t *testing.T, sb integration.Sandbox) {
+	for _, ent := range []bool{true, false} {
+		t.Run(fmt.Sprintf("ent=%v", ent), func(t *testing.T) {
+			t.Setenv("BUILDX_BAKE_ENTITLEMENTS_FS", strconv.FormatBool(ent))
+			dockerfile := []byte(`
+FROM scratch
+COPY foo /foo
+	`)
+			bakefile := []byte(`
+target "default" {
+}
+`)
+			dir := tmpdir(
+				t,
+				fstest.CreateFile("docker-bake.hcl", bakefile, 0600),
+				fstest.CreateFile("Dockerfile", dockerfile, 0600),
+				fstest.CreateFile("foo", []byte("foo"), 0600),
+			)
+
+			destDir := t.TempDir()
+
+			cmd := buildxCmd(sb, withDir(dir), withArgs("bake", "--progress=plain", "--set", "*.output=type=local,dest="+destDir))
+			out, err := cmd.CombinedOutput()
+			// existing directory via --set is always allowed
+			require.NoError(t, err, string(out))
+			require.FileExists(t, filepath.Join(destDir, "foo"))
+		})
+	}
+}
+
+func testBakeDefinitionNotExistingSubdirNoParallel(t *testing.T, sb integration.Sandbox) {
+	for _, ent := range []bool{true, false} {
+		t.Run(fmt.Sprintf("ent=%v", ent), func(t *testing.T) {
+			t.Setenv("BUILDX_BAKE_ENTITLEMENTS_FS", strconv.FormatBool(ent))
+			dockerfile := []byte(`
+FROM scratch
+COPY foo /foo
+	`)
+			bakefile := []byte(`
+target "default" {
+	output = ["type=local,dest=not/exists"]
+}
+`)
+			dir := tmpdir(
+				t,
+				fstest.CreateFile("docker-bake.hcl", bakefile, 0600),
+				fstest.CreateFile("Dockerfile", dockerfile, 0600),
+				fstest.CreateFile("foo", []byte("foo"), 0600),
+			)
+
+			cmd := buildxCmd(sb, withDir(dir), withArgs("bake", "--progress=plain"))
+			out, err := cmd.CombinedOutput()
+			// subdirs of working directory are always allowed
+			require.NoError(t, err, string(out))
+			require.FileExists(t, filepath.Join(dir, "not/exists/foo"))
+		})
+	}
+}
+
+func testBakeDefinitionNotExistingOutsideNoParallel(t *testing.T, sb integration.Sandbox) {
+	for _, ent := range []bool{true, false} {
+		t.Run(fmt.Sprintf("ent=%v", ent), func(t *testing.T) {
+			t.Setenv("BUILDX_BAKE_ENTITLEMENTS_FS", strconv.FormatBool(ent))
+			dockerfile := []byte(`
+FROM scratch
+COPY foo /foo
+	`)
+			destDir := t.TempDir()
+			bakefile := []byte(fmt.Sprintf(`
+target "default" {
+	output = ["type=local,dest=%s/not/exists"]
+}
+`, destDir))
+			dir := tmpdir(
+				t,
+				fstest.CreateFile("docker-bake.hcl", bakefile, 0600),
+				fstest.CreateFile("Dockerfile", dockerfile, 0600),
+				fstest.CreateFile("foo", []byte("foo"), 0600),
+			)
+
+			cmd := buildxCmd(sb, withDir(dir), withArgs("bake", "--progress=plain"))
+			out, err := cmd.CombinedOutput()
+			if ent {
+				require.Error(t, err, string(out))
+				require.Contains(t, string(out), "ERROR: additional privileges requested")
+			} else {
+				require.NoError(t, err, string(out))
+				require.FileExists(t, filepath.Join(destDir, "not/exists/foo"))
+			}
+		})
+	}
+}
+
+func testBakeDefinitionExistingOutsideNoParallel(t *testing.T, sb integration.Sandbox) {
+	for _, ent := range []bool{true, false} {
+		t.Run(fmt.Sprintf("ent=%v", ent), func(t *testing.T) {
+			t.Setenv("BUILDX_BAKE_ENTITLEMENTS_FS", strconv.FormatBool(ent))
+			dockerfile := []byte(`
+FROM scratch
+COPY foo /foo
+	`)
+			destDir := t.TempDir()
+			bakefile := []byte(fmt.Sprintf(`
+target "default" {
+	output = ["type=local,dest=%s"]
+}
+`, destDir))
+			dir := tmpdir(
+				t,
+				fstest.CreateFile("docker-bake.hcl", bakefile, 0600),
+				fstest.CreateFile("Dockerfile", dockerfile, 0600),
+				fstest.CreateFile("foo", []byte("foo"), 0600),
+			)
+
+			cmd := buildxCmd(sb, withDir(dir), withArgs("bake", "--progress=plain"))
+			out, err := cmd.CombinedOutput()
+			if ent {
+				require.Error(t, err, string(out))
+				require.Contains(t, string(out), "ERROR: additional privileges requested")
+			} else {
+				require.NoError(t, err, string(out))
+				require.FileExists(t, filepath.Join(destDir, "foo"))
+			}
+		})
+	}
+}
+
+func testBakeDefinitionSymlinkOutsideNoParallel(t *testing.T, sb integration.Sandbox) {
+	for _, ent := range []bool{true, false} {
+		t.Run(fmt.Sprintf("ent=%v", ent), func(t *testing.T) {
+			t.Setenv("BUILDX_BAKE_ENTITLEMENTS_FS", strconv.FormatBool(ent))
+			dockerfile := []byte(`
+FROM scratch
+COPY foo /foo
+	`)
+			destDir := t.TempDir()
+			bakefile := []byte(`
+target "default" {
+	output = ["type=local,dest=out"]
+}
+`)
+			dir := tmpdir(
+				t,
+				fstest.CreateFile("docker-bake.hcl", bakefile, 0600),
+				fstest.CreateFile("Dockerfile", dockerfile, 0600),
+				fstest.CreateFile("foo", []byte("foo"), 0600),
+				fstest.Symlink(destDir, "out"),
+			)
+
+			cmd := buildxCmd(sb, withDir(dir), withArgs("bake", "--progress=plain"))
+			out, err := cmd.CombinedOutput()
+			if ent {
+				require.Error(t, err, string(out))
+				require.Contains(t, string(out), "ERROR: additional privileges requested")
+			} else {
+				require.NoError(t, err, string(out))
+				require.FileExists(t, filepath.Join(destDir, "foo"))
+			}
+		})
+	}
+}
+
+func testBakeDefinitionSymlinkOutsideGrantedNoParallel(t *testing.T, sb integration.Sandbox) {
+	for _, ent := range []bool{true, false} {
+		t.Run(fmt.Sprintf("ent=%v", ent), func(t *testing.T) {
+			t.Setenv("BUILDX_BAKE_ENTITLEMENTS_FS", strconv.FormatBool(ent))
+			dockerfile := []byte(`
+FROM scratch
+COPY foo /foo
+	`)
+			destDir := t.TempDir()
+			bakefile := []byte(`
+target "default" {
+	output = ["type=local,dest=out"]
+}
+`)
+			dir := tmpdir(
+				t,
+				fstest.CreateFile("docker-bake.hcl", bakefile, 0600),
+				fstest.CreateFile("Dockerfile", dockerfile, 0600),
+				fstest.CreateFile("foo", []byte("foo"), 0600),
+				fstest.Symlink(destDir, "out"),
+			)
+
+			cmd := buildxCmd(sb, withDir(dir), withArgs("bake", "--progress=plain", "--allow", "fs.write="+destDir))
+			out, err := cmd.CombinedOutput()
+			require.NoError(t, err, string(out))
+			require.FileExists(t, filepath.Join(destDir, "foo"))
+		})
+	}
+}
+
 func testBakeUlimits(t *testing.T, sb integration.Sandbox) {
 	dockerfile := []byte(`
 FROM busybox AS build

util/buildflags/cache.go

@@ -14,6 +14,9 @@ import (
 func ParseCacheEntry(in []string) ([]*controllerapi.CacheOptionsEntry, error) {
 	outs := make([]*controllerapi.CacheOptionsEntry, 0, len(in))
 	for _, in := range in {
+		if in == "" {
+			continue
+		}
 		fields, err := csvvalue.Fields(in, nil)
 		if err != nil {
 			return nil, err

util/buildflags/export.go

@@ -19,6 +19,9 @@ func ParseExports(inp []string) ([]*controllerapi.ExportEntry, error) {
 		return nil, nil
 	}
 	for _, s := range inp {
+		if s == "" {
+			continue
+		}
 		fields, err := csvvalue.Fields(s, nil)
 		if err != nil {
 			return nil, err

util/buildflags/secrets.go

@@ -11,6 +11,9 @@ import (
 func ParseSecretSpecs(sl []string) ([]*controllerapi.Secret, error) {
 	fs := make([]*controllerapi.Secret, 0, len(sl))
 	for _, v := range sl {
+		if v == "" {
+			continue
+		}
 		s, err := parseSecret(v)
 		if err != nil {
 			return nil, err

util/buildflags/ssh.go

@@ -14,6 +14,9 @@ func ParseSSHSpecs(sl []string) ([]*controllerapi.SSH, error) {
 	}
 
 	for _, s := range sl {
+		if s == "" {
+			continue
+		}
 		parts := strings.SplitN(s, "=", 2)
 		out := controllerapi.SSH{
 			ID: parts[0],