exporter: fix src error cleanup and resolution

Release the mount or temp dir in CreateFS on every error path. An invalid
src could previously leak a mount for each failed export, and failing
attestation steps had the same leak.

Pass src to fs.RootPath unmodified so symlinks are resolved before "..",
and stop trimming whitespace from the value. Keep the underlying error
when reporting src failures so errors.Is still matches.

Add tests for mount release, mode=delete and platform-split=false with
src, and make the src error assertions work on Windows. Note in the
README that older daemons ignore src.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>

README.md

@@ -342,6 +342,9 @@ are resolved within the build result: a link pointing at `/etc` refers to `/etc`
 inside the result, never on the host running BuildKit. With a multi-platform
 build, `src` is applied to each platform.
 
+Use a BuildKit daemon that supports `src`. Older daemons ignore unknown exporter
+options and may export the entire build result instead.
+
 With a [multi-platform build](docs/multi-platform.md), a subfolder matching
 each target platform will be created in the destination directory:
 

client/client_export_local_test.go

@@ -1005,6 +1005,32 @@ func testExportLocalSource(t *testing.T, sb integration.Sandbox) {
 	)))
 }
 
+func testExportLocalSourceModeDelete(t *testing.T, sb integration.Sandbox) {
+	c, err := New(sb.Context(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	def, err := sourceTestState().Marshal(sb.Context())
+	require.NoError(t, err)
+
+	destDir := t.TempDir()
+	require.NoError(t, os.WriteFile(filepath.Join(destDir, "stale.txt"), []byte("stale"), 0600))
+	_, err = c.Solve(sb.Context(), def, SolveOpt{
+		Exports: []ExportEntry{{
+			Type:      ExporterLocal,
+			OutputDir: destDir,
+			Attrs:     map[string]string{"src": "/sub", "mode": "delete"},
+		}},
+	}, nil)
+	require.NoError(t, err)
+
+	require.NoError(t, fstest.CheckDirectoryEqualWithApplier(destDir, fstest.Apply(
+		fstest.CreateFile("nested.txt", []byte("nested"), 0600),
+		fstest.CreateDir("deeper", 0755),
+		fstest.CreateFile("deeper/deep.txt", []byte("deep"), 0600),
+	)))
+}
+
 func testExportLocalSourceNotFound(t *testing.T, sb integration.Sandbox) {
 	c, err := New(sb.Context(), sb.Address())
 	require.NoError(t, err)
@@ -1023,34 +1049,38 @@ func testExportLocalSourceNotFound(t *testing.T, sb integration.Sandbox) {
 			},
 		},
 	}, nil)
-	require.Error(t, err)
-	require.ErrorContains(t, err, "src=/nope no such file or directory")
+	require.ErrorContains(t, err, "src=/nope:")
 	// the mountpoint of the ref inside the daemon must never reach the client
 	require.NotContains(t, err.Error(), "buildkit-mount")
 }
 
 func testExportLocalSourceMultiPlatform(t *testing.T, sb integration.Sandbox) {
-	workers.CheckFeatureCompat(t, sb, workers.FeatureOCIExporter, workers.FeatureMultiPlatform)
+	testExportLocalSourceMultiPlatformSplit(t, sb, true)
+}
+
+func testExportLocalSourceNoPlatformSplit(t *testing.T, sb integration.Sandbox) {
+	testExportLocalSourceMultiPlatformSplit(t, sb, false)
+}
+
+func testExportLocalSourceMultiPlatformSplit(t *testing.T, sb integration.Sandbox, split bool) {
+	workers.CheckFeatureCompat(t, sb, workers.FeatureMultiPlatform)
 	c, err := New(sb.Context(), sb.Address())
 	require.NoError(t, err)
 	defer c.Close()
 
 	platformsToTest := []string{"linux/amd64", "linux/arm64"}
 
-	// every platform builds the same layout and differs only in file contents,
-	// so the exported platform directories can be compared against each other
 	frontend := func(ctx context.Context, c gateway.Client) (*gateway.Result, error) {
 		res := gateway.NewResult()
 		expPlatforms := &exptypes.Platforms{
 			Platforms: make([]exptypes.Platform, len(platformsToTest)),
 		}
 		for i, platform := range platformsToTest {
+			name := strings.ReplaceAll(platform, "/", "_") + ".txt"
 			st := llb.Scratch().
 				File(llb.Mkfile("top.txt", 0600, []byte("top"))).
 				File(llb.Mkdir("sub", 0755)).
-				File(llb.Mkfile("sub/nested.txt", 0600, []byte(platform))).
-				File(llb.Mkdir("sub/deeper", 0755)).
-				File(llb.Mkfile("sub/deeper/deep.txt", 0600, []byte(platform)))
+				File(llb.Mkfile("sub/"+name, 0600, []byte(platform)))
 
 			def, err := st.Marshal(ctx)
 			if err != nil {
@@ -1084,22 +1114,29 @@ func testExportLocalSourceMultiPlatform(t *testing.T, sb integration.Sandbox) {
 			{
 				Type:      ExporterLocal,
 				OutputDir: destDir,
-				Attrs:     map[string]string{"src": "/sub"},
+				Attrs:     map[string]string{"src": "/sub", "platform-split": fmt.Sprint(split)},
 			},
 		},
 	}, "", frontend, nil)
 	require.NoError(t, err)
 
-	// src is applied per platform: every platform directory holds the same
-	// re-rooted layout, with only the file contents telling them apart
 	for _, platform := range platformsToTest {
-		platDir := filepath.Join(destDir, strings.ReplaceAll(platform, "/", "_"))
-		require.NoError(t, fstest.CheckDirectoryEqualWithApplier(platDir, fstest.Apply(
-			fstest.CreateFile("nested.txt", []byte(platform), 0600),
-			fstest.CreateDir("deeper", 0755),
-			fstest.CreateFile("deeper/deep.txt", []byte(platform), 0600),
-		)), "unexpected content for %s", platform)
+		name := strings.ReplaceAll(platform, "/", "_") + ".txt"
+		outputDir := destDir
+		if split {
+			outputDir = filepath.Join(destDir, strings.ReplaceAll(platform, "/", "_"))
+			require.NoError(t, fstest.CheckDirectoryEqualWithApplier(outputDir, fstest.Apply(
+				fstest.CreateFile(name, []byte(platform), 0600),
+			)), "unexpected content for %s", platform)
+			continue
+		}
+		dt, err := os.ReadFile(filepath.Join(outputDir, name))
+		require.NoError(t, err)
+		require.Equal(t, platform, string(dt))
 	}
+	entries, err := os.ReadDir(destDir)
+	require.NoError(t, err)
+	require.Len(t, entries, len(platformsToTest))
 }
 
 func testExportTarSource(t *testing.T, sb integration.Sandbox) {

client/client_test.go

@@ -91,8 +91,10 @@ var allTests = []func(t *testing.T, sb integration.Sandbox){
 	testExportLocalNoPlatformSplit,
 	testExportLocalNoPlatformSplitOverwrite,
 	testExportLocalSource,
+	testExportLocalSourceModeDelete,
 	testExportLocalSourceNotFound,
 	testExportLocalSourceMultiPlatform,
+	testExportLocalSourceNoPlatformSplit,
 	testExportTarSource,
 	testExportTarPlatformIDSanitized,
 	testExporterTargetExists,

exporter/local/fs.go

@@ -76,11 +76,10 @@ func (c *CreateFSOpts) Load(opt map[string]string) (map[string]string, error) {
 				return nil, err
 			}
 		case keySource:
-			var src = strings.TrimSpace(v)
-			if src == "" {
-				return nil, errors.Errorf("empty value for %s omit it to export the entire filesystem", keySource)
+			if v == "" {
+				return nil, errors.Errorf("empty value for %s; omit it to export the entire filesystem", keySource)
 			}
-			c.Source = path.Join("/", src)
+			c.Source = v
 		default:
 			rest[k] = v
 		}
@@ -89,9 +88,11 @@ func (c *CreateFSOpts) Load(opt map[string]string) (map[string]string, error) {
 	return rest, nil
 }
 
-// Resolves source inside mountRoot and prevents path traversal
+// resolveSafeSource resolves source inside mountRoot and prevents path traversal.
 // An empty source returns mountRoot itself.
 func resolveSafeSource(mountRoot, source string) (fsutil.FS, error) {
+	// RootPath must see the original path so it can resolve symlinks before "..".
+	// The mounted ref must remain read-only while the returned FS is in use.
 	root, err := fs.RootPath(mountRoot, source)
 	if err != nil {
 		return nil, sourceError(err, source)
@@ -105,7 +106,7 @@ func resolveSafeSource(mountRoot, source string) (fsutil.FS, error) {
 	return outputFS, nil
 }
 
-// Reports err against the source the client asked for, hiding the
+// sourceError reports err against the source the client asked for, hiding the
 // daemon-side mountpoint that RootPath and NewFS name.
 func sourceError(err error, source string) error {
 	if source == "" {
@@ -113,12 +114,14 @@ func sourceError(err error, source string) error {
 	}
 	// the innermost *os.PathError carries the bare syscall error, with no path
 	cause := err
-	for e := err; e != nil; e = errors.Unwrap(e) {
-		if pe, ok := e.(*os.PathError); ok {
-			cause = pe.Err
+	for {
+		var pe *os.PathError
+		if !errors.As(cause, &pe) {
+			break
 		}
+		cause = pe.Err
 	}
-	return errors.Errorf("%s=%s %v", keySource, source, cause)
+	return errors.Wrapf(cause, "%s=%s", keySource, source)
 }
 
 func CreateFS(ctx context.Context, sessionID string, k string, ref cache.ImmutableRef, attestations []exporter.Attestation, defaultTime time.Time, isMap bool, opt CreateFSOpts) (fsutil.FS, func() error, error) {
@@ -149,6 +152,12 @@ func CreateFS(ctx context.Context, sessionID string, k string, ref cache.Immutab
 
 		cleanup = lm.Unmount
 	}
+	releaseOnError := true
+	defer func() {
+		if releaseOnError && cleanup != nil {
+			_ = cleanup()
+		}
+	}()
 
 	outputFS, err := resolveSafeSource(src, opt.Source)
 	if err != nil {
@@ -260,5 +269,6 @@ func CreateFS(ctx context.Context, sessionID string, k string, ref cache.Immutab
 		outputFS = staticfs.NewMergeFS(outputFS, stmtFS)
 	}
 
+	releaseOnError = false
 	return outputFS, cleanup, nil
 }

exporter/local/fs_test.go

@@ -1,102 +1,34 @@
 package local
 
 import (
+	"context"
 	"io/fs"
+	"os"
+	"path/filepath"
 	"sort"
+	"syscall"
 	"testing"
+	"time"
 
+	"github.com/containerd/containerd/v2/core/mount"
 	"github.com/containerd/continuity/fs/fstest"
+	"github.com/moby/buildkit/cache"
+	"github.com/moby/buildkit/session"
+	"github.com/moby/buildkit/snapshot"
+	"github.com/moby/sys/user"
 	"github.com/stretchr/testify/require"
 	"github.com/tonistiigi/fsutil"
 )
 
 func TestCreateFSOptsLoadSource(t *testing.T) {
-	tests := []struct {
-		name    string
-		attrs   map[string]string
-		want    string
-		wantErr string
-	}{
-		{
-			name:  "unset",
-			attrs: map[string]string{},
-			want:  "",
-		},
-		{
-			name:    "empty",
-			attrs:   map[string]string{keySource: ""},
-			wantErr: "empty value for src",
-		},
-		{
-			name:    "whitespace only",
-			attrs:   map[string]string{keySource: "   "},
-			wantErr: "empty value for src",
-		},
-		{
-			name:  "absolute",
-			attrs: map[string]string{keySource: "/app/build"},
-			want:  "/app/build",
-		},
-		{
-			name:  "trailing slash",
-			attrs: map[string]string{keySource: "/app/build/"},
-			want:  "/app/build",
-		},
-		{
-			name:  "relative is anchored to root",
-			attrs: map[string]string{keySource: "app/build"},
-			want:  "/app/build",
-		},
-		{
-			name:  "dot slash prefix",
-			attrs: map[string]string{keySource: "./app/build"},
-			want:  "/app/build",
-		},
-		{
-			name:  "redundant separators",
-			attrs: map[string]string{keySource: "/a//b/./c"},
-			want:  "/a/b/c",
-		},
-		{
-			name:  "dot is root",
-			attrs: map[string]string{keySource: "."},
-			want:  "/",
-		},
-		{
-			name:  "slash is root",
-			attrs: map[string]string{keySource: "/"},
-			want:  "/",
-		},
-		{
-			name:  "surrounding whitespace is trimmed",
-			attrs: map[string]string{keySource: "  /app  "},
-			want:  "/app",
-		},
-		{
-			// Clamping at the root matches what BuildKit already does for
-			// container-side paths: COPY --from, and both source and target of
-			// RUN --mount, accept ".." and clamp it the same way.
-			name:  "parent traversal is clamped to root",
-			attrs: map[string]string{keySource: "../../etc"},
-			want:  "/etc",
-		},
-	}
+	var opts CreateFSOpts
+	rest, err := opts.Load(map[string]string{keySource: "   "})
+	require.NoError(t, err)
+	require.Equal(t, "   ", opts.Source)
+	require.NotContains(t, rest, keySource)
 
-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			var opts CreateFSOpts
-			rest, err := opts.Load(tc.attrs)
-
-			if tc.wantErr != "" {
-				require.ErrorContains(t, err, tc.wantErr)
-				return
-			}
-
-			require.NoError(t, err)
-			require.Equal(t, tc.want, opts.Source)
-			require.NotContains(t, rest, keySource)
-		})
-	}
+	_, err = opts.Load(map[string]string{keySource: ""})
+	require.ErrorContains(t, err, "empty value for src")
 }
 
 // builds a container filesystem
@@ -135,7 +67,7 @@ func walkNames(t *testing.T, f fsutil.FS) []string {
 		if err != nil {
 			return err
 		}
-		names = append(names, p)
+		names = append(names, filepath.ToSlash(p))
 		return nil
 	}))
 	sort.Strings(names)
@@ -148,12 +80,18 @@ func TestResolveSafeSource(t *testing.T) {
 		source  string
 		want    []string
 		wantErr string
+		wantIs  error
 	}{
 		{
 			name:   "empty source exports the whole mount",
 			source: "",
 			want:   []string{"app", "etc", "etc/inside.txt", "rel", "sub", "sub/nested.txt", "top.txt"},
 		},
+		{
+			name:   "relative source is anchored to mount",
+			source: "sub",
+			want:   []string{"nested.txt"},
+		},
 		{
 			name:   "subdirectory is re-rooted",
 			source: "/sub",
@@ -170,7 +108,7 @@ func TestResolveSafeSource(t *testing.T) {
 			want:   []string{"nested.txt"},
 		},
 		{
-			// same clamping as COPY --from and RUN --mount, see the Load test
+			// Parent traversal cannot escape the mounted result.
 			name:   "parent traversal is clamped to the mount",
 			source: "../../etc",
 			want:   []string{"inside.txt"},
@@ -178,12 +116,14 @@ func TestResolveSafeSource(t *testing.T) {
 		{
 			name:    "missing path fails",
 			source:  "/nope",
-			wantErr: "src=/nope no such file or directory",
+			wantErr: "src=/nope:",
+			wantIs:  os.ErrNotExist,
 		},
 		{
 			name:    "file is not a directory",
 			source:  "/top.txt",
-			wantErr: "src=/top.txt not a directory",
+			wantErr: "src=/top.txt:",
+			wantIs:  syscall.ENOTDIR,
 		},
 	}
 
@@ -191,7 +131,8 @@ func TestResolveSafeSource(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			outputFS, err := resolveSafeSource(newRootfs(t), tc.source)
 			if tc.wantErr != "" {
-				require.EqualError(t, err, tc.wantErr)
+				require.ErrorContains(t, err, tc.wantErr)
+				require.ErrorIs(t, err, tc.wantIs)
 				return
 			}
 			require.NoError(t, err)
@@ -199,3 +140,74 @@ func TestResolveSafeSource(t *testing.T) {
 		})
 	}
 }
+
+func TestSourceResolvesParentAfterSymlink(t *testing.T) {
+	root := t.TempDir()
+	require.NoError(t, fstest.Apply(
+		fstest.CreateFile("top.txt", []byte("top"), 0600),
+		fstest.CreateDir("sub", 0755),
+		fstest.CreateFile("sub/nested.txt", []byte("nested"), 0600),
+		fstest.CreateDir("sub/inner", 0755),
+		fstest.Symlink("/sub/inner", "link"),
+	).Apply(root))
+
+	var opts CreateFSOpts
+	_, err := opts.Load(map[string]string{keySource: "/link/.."})
+	require.NoError(t, err)
+
+	outputFS, err := resolveSafeSource(root, opts.Source)
+	require.NoError(t, err)
+	require.Equal(t, []string{"inner", "nested.txt"}, walkNames(t, outputFS))
+}
+
+func TestCreateFSReleasesTempDirOnSourceError(t *testing.T) {
+	tempDir := t.TempDir()
+	t.Setenv("TMPDIR", tempDir)
+	t.Setenv("TMP", tempDir)
+	t.Setenv("TEMP", tempDir)
+	require.Equal(t, tempDir, os.TempDir())
+
+	_, cleanup, err := CreateFS(t.Context(), "", "", nil, nil, time.Time{}, false, CreateFSOpts{Source: "/missing"})
+	require.ErrorIs(t, err, os.ErrNotExist)
+	require.Nil(t, cleanup)
+
+	entries, err := os.ReadDir(tempDir)
+	require.NoError(t, err)
+	require.Empty(t, entries)
+}
+
+type sourceTestRef struct {
+	// CreateFS only calls Mount; the remaining methods are not needed here.
+	cache.ImmutableRef
+	mount snapshot.Mountable
+}
+
+func (r sourceTestRef) Mount(context.Context, bool, session.Group) (snapshot.Mountable, error) {
+	return r.mount, nil
+}
+
+type sourceTestMountable struct {
+	root     string
+	releases int
+}
+
+func (m *sourceTestMountable) Mount() ([]mount.Mount, func() error, error) {
+	return []mount.Mount{{Type: "bind", Source: m.root}}, func() error {
+		m.releases++
+		return nil
+	}, nil
+}
+
+func (m *sourceTestMountable) IdentityMapping() *user.IdentityMapping {
+	return nil
+}
+
+func TestCreateFSReleasesMountOnSourceError(t *testing.T) {
+	m := &sourceTestMountable{root: t.TempDir()}
+	ref := sourceTestRef{mount: m}
+
+	_, cleanup, err := CreateFS(t.Context(), "", "", ref, nil, time.Time{}, false, CreateFSOpts{Source: "/missing"})
+	require.ErrorIs(t, err, os.ErrNotExist)
+	require.Nil(t, cleanup)
+	require.Equal(t, 1, m.releases)
+}