Merge pull request #898 from docker/fix/tarball-forward-slash-paths

fix: use path.Join instead of filepath.Join for tar entry names

Author: ericcurtin

pkg/distribution/tarball/target.go

@@ -5,7 +5,7 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"path/filepath"
+	"path"
 
 	"github.com/docker/model-runner/pkg/distribution/internal/progress"
 	"github.com/docker/model-runner/pkg/distribution/oci"
@@ -68,7 +68,7 @@ func (t *Target) Write(ctx context.Context, mdl types.ModelArtifact, progressWri
 		return err
 	}
 	if err = tw.WriteHeader(&tar.Header{
-		Name: filepath.Join("blobs", cn.Algorithm, cn.Hex),
+		Name: path.Join("blobs", cn.Algorithm, cn.Hex),
 		Mode: 0666,
 		Size: int64(len(rcf)),
 	}); err != nil {
@@ -97,15 +97,15 @@ func (t *Target) addLayer(layer oci.Layer, tw *tar.Writer, progressWriter io.Wri
 	if err != nil {
 		return fmt.Errorf("get layer diffID: %w", err)
 	}
-	if err := t.ensureDir(filepath.Join("blobs", diffID.Algorithm), tw); err != nil {
+	if err := t.ensureDir(path.Join("blobs", diffID.Algorithm), tw); err != nil {
 		return err
 	}
 	sz, err := layer.Size()
 	if err != nil {
 		return fmt.Errorf("get layer size: %w", err)
 	}
 	if err = tw.WriteHeader(&tar.Header{
-		Name: filepath.Join("blobs", diffID.Algorithm, diffID.Hex),
+		Name: path.Join("blobs", diffID.Algorithm, diffID.Hex),
 		Mode: 0666,
 		Size: sz,
 	}); err != nil {
@@ -138,15 +138,16 @@ func (t *Target) addLayer(layer oci.Layer, tw *tar.Writer, progressWriter io.Wri
 	return nil
 }
 
-func (t *Target) ensureDir(path string, tw *tar.Writer) error {
-	if _, ok := t.dirs[path]; !ok {
+func (t *Target) ensureDir(p string, tw *tar.Writer) error {
+	if _, ok := t.dirs[p]; !ok {
 		if err := tw.WriteHeader(&tar.Header{
-			Name:     path,
+			Name:     p,
 			Typeflag: tar.TypeDir,
+			Mode:     0755,
 		}); err != nil {
-			return fmt.Errorf("add dir entry %q: %w", path, err)
+			return fmt.Errorf("add dir entry %q: %w", p, err)
 		}
 	}
-	t.dirs[path] = struct{}{}
+	t.dirs[p] = struct{}{}
 	return nil
 }

pkg/distribution/tarball/target_test.go

@@ -6,6 +6,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/docker/model-runner/pkg/distribution/builder"
@@ -70,6 +71,50 @@ func TestTarget(t *testing.T) {
 	hasFile(t, tr, "manifest.json", manifestContents)
 }
 
+// TestTargetEntryNamesUseForwardSlashes verifies that all tar entry names
+// produced by Target.Write use forward slashes, even for models with multiple
+// layers (e.g., GGUF + chat template).
+//
+// This is a regression test for https://github.com/docker/model-runner/issues/894
+// where filepath.Join on Windows produced backslash-separated entry names
+// (e.g., "blobs\sha256\hex"), causing the daemon reader to skip the blobs.
+func TestTargetEntryNamesUseForwardSlashes(t *testing.T) {
+	b, err := builder.FromPath(filepath.Join("..", "assets", "dummy.gguf"))
+	if err != nil {
+		t.Fatalf("Failed to create builder from GGUF: %v", err)
+	}
+	b, err = b.WithChatTemplateFile(filepath.Join("..", "assets", "template.jinja"))
+	if err != nil {
+		t.Fatalf("Failed to add chat template: %v", err)
+	}
+
+	var buf bytes.Buffer
+	target, err := tarball.NewTarget(&buf)
+	if err != nil {
+		t.Fatalf("Failed to create target: %v", err)
+	}
+	if err := target.Write(t.Context(), b.Model(), nil); err != nil {
+		t.Fatalf("Failed to write model: %v", err)
+	}
+
+	// Read all tar entries and verify none contain backslashes.
+	tr := tar.NewReader(&buf)
+	for {
+		hdr, err := tr.Next()
+		if err == io.EOF {
+			break
+		}
+		if err != nil {
+			t.Fatalf("Failed to read tar entry: %v", err)
+		}
+		if strings.Contains(hdr.Name, "\\") {
+			t.Errorf("Tar entry name contains backslash: %q — "+
+				"tar entry names must use forward slashes for cross-platform compatibility "+
+				"(see https://github.com/docker/model-runner/issues/894)", hdr.Name)
+		}
+	}
+}
+
 func hasFile(t *testing.T, tr *tar.Reader, name string, contents []byte) {
 	hdr, err := tr.Next()
 	if err != nil {