diff --git a/client/client_export_local_test.go b/client/client_export_local_test.go index b162f5b20..1cbf98c92 100644 --- a/client/client_export_local_test.go +++ b/client/client_export_local_test.go @@ -11,6 +11,7 @@ import ( "path" "path/filepath" "runtime" + "sort" "strings" "testing" @@ -443,6 +444,90 @@ func testExportLocalNoPlatformSplitOverwrite(t *testing.T, sb integration.Sandbo require.ErrorContains(t, err, "when split option is disabled") } +func testExportTarPlatformIDSanitized(t *testing.T, sb integration.Sandbox) { + workers.CheckFeatureCompat(t, sb, workers.FeatureOCIExporter, workers.FeatureMultiPlatform) + c, err := New(sb.Context(), sb.Address()) + require.NoError(t, err) + defer c.Close() + + const platformID = `..\buildkit-outside` + platform := platforms.DefaultSpec() + + frontend := func(ctx context.Context, c gateway.Client) (*gateway.Result, error) { + st := llb.Scratch().File( + llb.Mkfile("payload.txt", 0600, []byte("payload")), + ) + + def, err := st.Marshal(ctx) + if err != nil { + return nil, err + } + + r, err := c.Solve(ctx, gateway.SolveRequest{ + Definition: def.ToPB(), + }) + if err != nil { + return nil, err + } + + ref, err := r.SingleRef() + if err != nil { + return nil, err + } + + res := gateway.NewResult() + res.AddRef(platformID, ref) + + dt, err := json.Marshal(&exptypes.Platforms{ + Platforms: []exptypes.Platform{{ + ID: platformID, + Platform: platform, + }}, + }) + if err != nil { + return nil, err + } + res.AddMeta(exptypes.ExporterPlatformsKey, dt) + + return res, nil + } + + outW := bytes.NewBuffer(nil) + _, err = c.Build(sb.Context(), SolveOpt{ + Exports: []ExportEntry{ + { + Type: ExporterTar, + Output: fixedWriteCloser(&iohelper.NopWriteCloser{Writer: outW}), + }, + }, + }, "", frontend, nil) + require.NoError(t, err) + + m, err := testutil.ReadTarToMap(outW.Bytes(), false) + require.NoError(t, err) + + for name := range m { + require.Falsef(t, strings.HasPrefix(name, "../") || + strings.HasPrefix(name, `..\`) || + strings.Contains(name, `/../`) || + strings.Contains(name, `\..\`) || + strings.Contains(name, `\`) || + strings.Contains(name, ":"), + "tar exporter emitted unsafe platform path %q", name) + } + + tarPaths := make([]string, 0, len(m)) + for name := range m { + tarPaths = append(tarPaths, name) + } + sort.Strings(tarPaths) + + expectedPath := path.Join(".._buildkit-outside", integration.UnixOrWindows("payload.txt", "Files/payload.txt")) + item := m[expectedPath] + require.NotNilf(t, item, "expected sanitized tar path %q in %v", expectedPath, tarPaths) + require.Equal(t, "payload", string(item.Data)) +} + func testExporterTargetExists(t *testing.T, sb integration.Sandbox) { workers.CheckFeatureCompat(t, sb, workers.FeatureOCIExporter) c, err := New(sb.Context(), sb.Address()) diff --git a/client/client_test.go b/client/client_test.go index 53da6223b..f5843dc6c 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -89,6 +89,7 @@ var allTests = []func(t *testing.T, sb integration.Sandbox){ testExportLocalModeDeleteMultiPlatformKeepsAllPlatforms, testExportLocalNoPlatformSplit, testExportLocalNoPlatformSplitOverwrite, + testExportTarPlatformIDSanitized, testExporterTargetExists, testMultipleExporters, testSessionExporter, diff --git a/exporter/local/export.go b/exporter/local/export.go index ef1b00ae1..3cf9218c6 100644 --- a/exporter/local/export.go +++ b/exporter/local/export.go @@ -3,7 +3,6 @@ package local import ( "context" "os" - "strings" "sync" "time" @@ -127,7 +126,7 @@ func (e *localExporterInstance) Export(ctx context.Context, inp *exporter.Source platformDirStat := func(k string, opt CreateFSOpts) *fstypes.Stat { st := &fstypes.Stat{ Mode: uint32(os.ModeDir | 0755), - Path: strings.ReplaceAll(k, "/", "_"), + Path: PlatformIDToPath(k), } if opt.Epoch != nil && opt.Epoch.Value != nil { st.ModTime = opt.Epoch.Value.UnixNano() diff --git a/exporter/local/fs.go b/exporter/local/fs.go index 0f5186a29..a47e69345 100644 --- a/exporter/local/fs.go +++ b/exporter/local/fs.go @@ -199,7 +199,7 @@ func CreateFS(ctx context.Context, sessionID string, k string, ref cache.Immutab if addPlatformToFilename { nameExt := path.Ext(name) namBase := strings.TrimSuffix(name, nameExt) - name = fmt.Sprintf("%s.%s%s", namBase, strings.ReplaceAll(k, "/", "_"), nameExt) + name = fmt.Sprintf("%s.%s%s", namBase, PlatformIDToPath(k), nameExt) } if _, ok := names[name]; ok { return nil, nil, errors.Errorf("duplicate attestation path name %s", name) diff --git a/exporter/local/platform.go b/exporter/local/platform.go new file mode 100644 index 000000000..c8acd5afa --- /dev/null +++ b/exporter/local/platform.go @@ -0,0 +1,14 @@ +package local + +import "strings" + +// PlatformIDToPath maps an exporter platform ID to a single path component. +func PlatformIDToPath(id string) string { + id = strings.NewReplacer("/", "_", `\`, "_", ":", "_").Replace(id) + switch id { + case ".", "..": + return strings.Repeat("_", len(id)) + default: + return id + } +} diff --git a/exporter/local/platform_test.go b/exporter/local/platform_test.go new file mode 100644 index 000000000..a258068c6 --- /dev/null +++ b/exporter/local/platform_test.go @@ -0,0 +1,50 @@ +package local + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestPlatformIDToPath(t *testing.T) { + for _, tc := range []struct { + name string + id string + want string + }{ + { + name: "slashes", + id: "linux/amd64", + want: "linux_amd64", + }, + { + name: "windows separators", + id: `..\buildkit-outside`, + want: ".._buildkit-outside", + }, + { + name: "drive separator", + id: `windows/amd64:C`, + want: "windows_amd64_C", + }, + { + name: "dot", + id: ".", + want: "_", + }, + { + name: "dot dot", + id: "..", + want: "__", + }, + { + name: "embedded dots", + id: "linux.v2/amd64", + want: "linux.v2_amd64", + }, + } { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.want, PlatformIDToPath(tc.id)) + }) + } +} diff --git a/exporter/tar/export.go b/exporter/tar/export.go index ef0e41330..796bc8e80 100644 --- a/exporter/tar/export.go +++ b/exporter/tar/export.go @@ -4,7 +4,6 @@ import ( "context" "os" "slices" - "strings" "time" "github.com/moby/buildkit/cache" @@ -109,7 +108,7 @@ func (e *localExporterInstance) Export(ctx context.Context, inp *exporter.Source st := &fstypes.Stat{ Mode: uint32(os.ModeDir | 0755), - Path: strings.ReplaceAll(k, "/", "_"), + Path: local.PlatformIDToPath(k), } if opt.Epoch != nil && opt.Epoch.Value != nil { st.ModTime = opt.Epoch.Value.UnixNano()