From 94d5fe1949ab9c167125088e843b36afc5213c6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20L=C3=B3pez=20Luna?= Date: Thu, 16 Oct 2025 13:51:29 +0200 Subject: [PATCH 01/16] feat(builder): add support for directory tar archives in artifact creation --- pkg/distribution/builder/builder.go | 12 ++++++++++++ pkg/distribution/types/config.go | 3 +++ 2 files changed, 15 insertions(+) diff --git a/pkg/distribution/builder/builder.go b/pkg/distribution/builder/builder.go index 9b6be8c5f..f64dd7ab4 100644 --- a/pkg/distribution/builder/builder.go +++ b/pkg/distribution/builder/builder.go @@ -102,6 +102,18 @@ func (b *Builder) WithConfigArchive(path string) (*Builder, error) { }, nil } +// WithDirTar adds a directory tar archive to the artifact. +// Multiple directory tar archives can be added by calling this method multiple times. +func (b *Builder) WithDirTar(path string) (*Builder, error) { + dirTarLayer, err := partial.NewLayer(path, types.MediaTypeDirTar) + if err != nil { + return nil, fmt.Errorf("dir tar layer from %q: %w", path, err) + } + return &Builder{ + model: mutate.AppendLayers(b.model, dirTarLayer), + }, nil +} + // Target represents a build target type Target interface { Write(context.Context, types.ModelArtifact, io.Writer) error diff --git a/pkg/distribution/types/config.go b/pkg/distribution/types/config.go index adce5d9f8..a5bc070dd 100644 --- a/pkg/distribution/types/config.go +++ b/pkg/distribution/types/config.go @@ -23,6 +23,9 @@ const ( // MediaTypeVLLMConfigArchive indicates a tar archive containing vLLM-specific config files. MediaTypeVLLMConfigArchive = types.MediaType("application/vnd.docker.ai.vllm.config.tar") + // MediaTypeDirTar indicates a tar archive containing a directory with its structure preserved. + MediaTypeDirTar = types.MediaType("application/vnd.docker.ai.dir.tar") + // MediaTypeLicense indicates a plain text file containing a license MediaTypeLicense = types.MediaType("application/vnd.docker.ai.license") From 50414502b12e2aa84ce7c6e7fabb069969b5766c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20L=C3=B3pez=20Luna?= Date: Thu, 16 Oct 2025 14:00:54 +0200 Subject: [PATCH 02/16] feat(packaging): add tests for directory tar archive creation and unpacking --- pkg/distribution/internal/bundle/unpack.go | 69 ++++++++++++ pkg/distribution/packaging/dirtar_test.go | 124 +++++++++++++++++++++ 2 files changed, 193 insertions(+) create mode 100644 pkg/distribution/packaging/dirtar_test.go diff --git a/pkg/distribution/internal/bundle/unpack.go b/pkg/distribution/internal/bundle/unpack.go index 6609063a0..780a21ad3 100644 --- a/pkg/distribution/internal/bundle/unpack.go +++ b/pkg/distribution/internal/bundle/unpack.go @@ -60,6 +60,11 @@ func Unpack(dir string, model types.Model) (*Bundle, error) { } } + // Unpack directory tar archives (can be multiple) + if err := unpackDirTarArchives(bundle, model); err != nil { + return nil, fmt.Errorf("unpack directory tar archives: %w", err) + } + // Always create the runtime config if err := unpackRuntimeConfig(bundle, model); err != nil { return nil, fmt.Errorf("add config.json to runtime bundle: %w", err) @@ -230,6 +235,70 @@ func unpackConfigArchive(bundle *Bundle, mdl types.Model) error { return nil } +func unpackDirTarArchives(bundle *Bundle, mdl types.Model) error { + // Cast to ModelArtifact to access Layers() method + artifact, ok := mdl.(types.ModelArtifact) + if !ok { + // If it's not a ModelArtifact, there are no layers to extract + return nil + } + + // Get all layers from the model + layers, err := artifact.Layers() + if err != nil { + return fmt.Errorf("get model layers: %w", err) + } + + modelDir := filepath.Join(bundle.dir, ModelSubdir) + + // Iterate through layers and extract directory tar archives + for _, layer := range layers { + mediaType, err := layer.MediaType() + if err != nil { + continue + } + + // Check if this is a directory tar layer + if mediaType != types.MediaTypeDirTar { + continue + } + + // Get the layer as a compressed blob + compressed, err := layer.Compressed() + if err != nil { + return fmt.Errorf("get compressed layer: %w", err) + } + + // Create a temp file to store the layer + tempFile, err := os.CreateTemp("", "dir-tar-*.tar") + if err != nil { + compressed.Close() + return fmt.Errorf("create temp file: %w", err) + } + tempPath := tempFile.Name() + + // Copy the layer to the temp file + if _, err := io.Copy(tempFile, compressed); err != nil { + tempFile.Close() + compressed.Close() + return fmt.Errorf("copy layer to temp file: %w", err) + } + tempFile.Close() + compressed.Close() + + // Extract the tar archive into the model subdirectory + if err := extractTarArchive(tempPath, modelDir); err != nil { + os.Remove(tempPath) + return fmt.Errorf("extract directory tar archive: %w", err) + } + + // Clean up temp file immediately after extraction + os.Remove(tempPath) + } + + return nil +} + // validatePathWithinDirectory checks if targetPath is within baseDir to prevent directory traversal attacks. // It uses filepath.IsLocal() to provide robust security against // various directory traversal attempts including edge cases like empty paths, ".", "..", symbolic links, etc. diff --git a/pkg/distribution/packaging/dirtar_test.go b/pkg/distribution/packaging/dirtar_test.go new file mode 100644 index 000000000..b7cfb9c1c --- /dev/null +++ b/pkg/distribution/packaging/dirtar_test.go @@ -0,0 +1,124 @@ +package packaging + +import ( + "archive/tar" + "io" + "os" + "path/filepath" + "testing" +) + +func TestCreateDirTarArchive(t *testing.T) { + // Create a temporary directory with some test files + tempDir, err := os.MkdirTemp("", "dirtar-test-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tempDir) + + // Create test directory structure + testDir := filepath.Join(tempDir, "test_directory") + if err := os.MkdirAll(filepath.Join(testDir, "subdir"), 0755); err != nil { + t.Fatalf("Failed to create test directory: %v", err) + } + + // Create test files + testFiles := map[string]string{ + "file1.txt": "content1", + "subdir/file2.txt": "content2", + } + + for relPath, content := range testFiles { + fullPath := filepath.Join(testDir, relPath) + if err := os.WriteFile(fullPath, []byte(content), 0644); err != nil { + t.Fatalf("Failed to write test file %s: %v", relPath, err) + } + } + + // Create tar archive + tarPath, err := CreateDirectoryTarArchive(testDir) + if err != nil { + t.Fatalf("CreateDirectoryTarArchive failed: %v", err) + } + defer os.Remove(tarPath) + + // Verify tar archive exists + if _, err := os.Stat(tarPath); os.IsNotExist(err) { + t.Fatal("Tar archive was not created") + } + + // Read and verify tar contents + file, err := os.Open(tarPath) + if err != nil { + t.Fatalf("Failed to open tar archive: %v", err) + } + defer file.Close() + + tr := tar.NewReader(file) + foundFiles := make(map[string]bool) + + for { + header, err := tr.Next() + if err == io.EOF { + break + } + if err != nil { + t.Fatalf("Failed to read tar header: %v", err) + } + + foundFiles[header.Name] = true + + // Verify it's within the test_directory structure + if header.Typeflag == tar.TypeReg { + // Read file content + content, err := io.ReadAll(tr) + if err != nil { + t.Fatalf("Failed to read file content: %v", err) + } + + // Verify content matches + expectedPath := header.Name[len("test_directory/"):] + if expectedContent, ok := testFiles[expectedPath]; ok { + if string(content) != expectedContent { + t.Errorf("File %s content mismatch: got %q, want %q", expectedPath, string(content), expectedContent) + } + } + } + } + + // Verify all expected entries are present + expectedEntries := []string{ + "test_directory", + "test_directory/file1.txt", + "test_directory/subdir", + "test_directory/subdir/file2.txt", + } + + for _, entry := range expectedEntries { + if !foundFiles[entry] { + t.Errorf("Expected entry %q not found in tar archive", entry) + } + } +} + +func TestCreateDirTarArchive_NonExistentDir(t *testing.T) { + _, err := CreateDirectoryTarArchive("/nonexistent/directory") + if err == nil { + t.Error("Expected error for non-existent directory, got nil") + } +} + +func TestCreateDirTarArchive_NotADirectory(t *testing.T) { + // Create a temporary file + tempFile, err := os.CreateTemp("", "not-a-dir-*") + if err != nil { + t.Fatalf("Failed to create temp file: %v", err) + } + defer os.Remove(tempFile.Name()) + tempFile.Close() + + _, err = CreateDirectoryTarArchive(tempFile.Name()) + if err == nil { + t.Error("Expected error for file path instead of directory, got nil") + } +} From 7df30ba5420c2653735d288bafc423d2f69cea32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20L=C3=B3pez=20Luna?= Date: Thu, 16 Oct 2025 14:01:47 +0200 Subject: [PATCH 03/16] feat(package): add support for packaging directories as tar archives --- cmd/cli/commands/package.go | 58 ++++++++++++ pkg/distribution/packaging/safetensors.go | 102 ++++++++++++++++++++++ 2 files changed, 160 insertions(+) diff --git a/cmd/cli/commands/package.go b/cmd/cli/commands/package.go index fa1683f30..bb2c08695 100644 --- a/cmd/cli/commands/package.go +++ b/cmd/cli/commands/package.go @@ -123,6 +123,7 @@ func newPackagedCmd() *cobra.Command { c.Flags().StringVar(&opts.safetensorsDir, "safetensors-dir", "", "absolute path to directory containing safetensors files and config") c.Flags().StringVar(&opts.chatTemplatePath, "chat-template", "", "absolute path to chat template file (must be Jinja format)") c.Flags().StringArrayVarP(&opts.licensePaths, "license", "l", nil, "absolute path to a license file") + c.Flags().StringArrayVar(&opts.dirTarPaths, "dir-tar", nil, "relative path to directory to package as tar (can be specified multiple times)") c.Flags().BoolVar(&opts.push, "push", false, "push to registry (if not set, the model is loaded into the Model Runner content store)") c.Flags().Uint64Var(&opts.contextSize, "context-size", 0, "context size in tokens") return c @@ -134,6 +135,7 @@ type packageOptions struct { ggufPath string safetensorsDir string licensePaths []string + dirTarPaths []string push bool tag string } @@ -213,6 +215,62 @@ func packageModel(cmd *cobra.Command, opts packageOptions) error { } } + // Process directory tar archives + var tempDirTarFiles []string + if len(opts.dirTarPaths) > 0 { + // Determine base directory for resolving relative paths + var baseDir string + if opts.safetensorsDir != "" { + baseDir = opts.safetensorsDir + } else { + // For GGUF, use the directory containing the GGUF file + baseDir = filepath.Dir(opts.ggufPath) + } + + for _, relDirPath := range opts.dirTarPaths { + // Resolve the full directory path + fullDirPath := filepath.Join(baseDir, relDirPath) + fullDirPath = filepath.Clean(fullDirPath) + + // Verify the directory exists + info, err := os.Stat(fullDirPath) + if err != nil { + return fmt.Errorf("cannot access directory %q (resolved from %q): %w", fullDirPath, relDirPath, err) + } + if !info.IsDir() { + return fmt.Errorf("path %q is not a directory", fullDirPath) + } + + cmd.PrintErrf("Creating tar archive for directory %q\n", relDirPath) + tempTarPath, err := packaging.CreateDirectoryTarArchive(fullDirPath) + if err != nil { + // Clean up any temp files created so far + for _, tempFile := range tempDirTarFiles { + os.Remove(tempFile) + } + return fmt.Errorf("create tar archive for directory %q: %w", relDirPath, err) + } + tempDirTarFiles = append(tempDirTarFiles, tempTarPath) + + cmd.PrintErrf("Adding directory tar archive from %q\n", relDirPath) + pkg, err = pkg.WithDirTar(tempTarPath) + if err != nil { + // Clean up temp files + for _, tempFile := range tempDirTarFiles { + os.Remove(tempFile) + } + return fmt.Errorf("add directory tar: %w", err) + } + } + + // Schedule cleanup of temp tar files after build completes + defer func() { + for _, tempFile := range tempDirTarFiles { + os.Remove(tempFile) + } + }() + } + if opts.push { cmd.PrintErrln("Pushing model to registry...") } else { diff --git a/pkg/distribution/packaging/safetensors.go b/pkg/distribution/packaging/safetensors.go index 379fb4a38..cf14c977f 100644 --- a/pkg/distribution/packaging/safetensors.go +++ b/pkg/distribution/packaging/safetensors.go @@ -144,3 +144,105 @@ func addFileToTar(tw *tar.Writer, filePath string) error { return nil } + +// CreateDirectoryTarArchive creates a temporary tar archive containing the specified directory +// with its structure preserved. It returns the path to the temporary tar file and any error encountered. +// The caller is responsible for removing the temporary file when done. +func CreateDirectoryTarArchive(dirPath string) (string, error) { + // Verify directory exists + info, err := os.Stat(dirPath) + if err != nil { + return "", fmt.Errorf("stat directory: %w", err) + } + if !info.IsDir() { + return "", fmt.Errorf("path is not a directory: %s", dirPath) + } + + // Create temp file + tmpFile, err := os.CreateTemp("", "dir-tar-*.tar") + if err != nil { + return "", fmt.Errorf("create temp file: %w", err) + } + tmpPath := tmpFile.Name() + + // Track success to determine if we should clean up the temp file + shouldKeepTempFile := false + defer func() { + if !shouldKeepTempFile { + os.Remove(tmpPath) + } + }() + + // Create tar writer + tw := tar.NewWriter(tmpFile) + + // Walk the directory tree + err = filepath.Walk(dirPath, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + // Create tar header + header, err := tar.FileInfoHeader(info, "") + if err != nil { + return fmt.Errorf("create tar header for %s: %w", path, err) + } + + // Compute relative path from the parent of dirPath + relPath, err := filepath.Rel(filepath.Dir(dirPath), path) + if err != nil { + return fmt.Errorf("compute relative path: %w", err) + } + + // Use forward slashes for tar archive paths + header.Name = filepath.ToSlash(relPath) + + // Write header + if err := tw.WriteHeader(header); err != nil { + return fmt.Errorf("write tar header: %w", err) + } + + // If it's a file, write its contents + if !info.IsDir() { + file, err := os.Open(path) + if err != nil { + return fmt.Errorf("open file %s: %w", path, err) + } + + // Copy file contents + _, copyErr := io.Copy(tw, file) + + // Close immediately to avoid file descriptor exhaustion in large directories + closeErr := file.Close() + + if copyErr != nil { + return fmt.Errorf("write tar content for %s: %w", path, copyErr) + } + if closeErr != nil { + return fmt.Errorf("close file %s: %w", path, closeErr) + } + } + + return nil + }) + + if err != nil { + tw.Close() + tmpFile.Close() + return "", fmt.Errorf("walk directory: %w", err) + } + + // Close tar writer + if err := tw.Close(); err != nil { + tmpFile.Close() + return "", fmt.Errorf("close tar writer: %w", err) + } + + // Close temp file + if err := tmpFile.Close(); err != nil { + return "", fmt.Errorf("close temp file: %w", err) + } + + shouldKeepTempFile = true + return tmpPath, nil +} From 91ca308b1df1ac46f697b737ddf8aa77731035ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20L=C3=B3pez=20Luna?= Date: Thu, 16 Oct 2025 14:14:01 +0200 Subject: [PATCH 04/16] feat(packaging): implement CreateDirectoryTarArchive function for creating tar archives from directories --- pkg/distribution/packaging/dirtar.go | 104 ++++++++++++++++++++++ pkg/distribution/packaging/safetensors.go | 102 --------------------- 2 files changed, 104 insertions(+), 102 deletions(-) create mode 100644 pkg/distribution/packaging/dirtar.go diff --git a/pkg/distribution/packaging/dirtar.go b/pkg/distribution/packaging/dirtar.go new file mode 100644 index 000000000..6896dc31d --- /dev/null +++ b/pkg/distribution/packaging/dirtar.go @@ -0,0 +1,104 @@ +package packaging + +import ( + "archive/tar" + "fmt" + "io" + "os" + "path/filepath" +) + +// CreateDirectoryTarArchive creates a temporary tar archive containing the specified directory +// with its structure preserved. It returns the path to the temporary tar file and any error encountered. +// The caller is responsible for removing the temporary file when done. +func CreateDirectoryTarArchive(dirPath string) (string, error) { + // Verify directory exists + info, err := os.Stat(dirPath) + if err != nil { + return "", fmt.Errorf("stat directory: %w", err) + } + if !info.IsDir() { + return "", fmt.Errorf("path is not a directory: %s", dirPath) + } + + // Create temp file + tmpFile, err := os.CreateTemp("", "dir-tar-*.tar") + if err != nil { + return "", fmt.Errorf("create temp file: %w", err) + } + tmpPath := tmpFile.Name() + + // Track success to determine if we should clean up the temp file + shouldKeepTempFile := false + defer func() { + if !shouldKeepTempFile { + os.Remove(tmpPath) + } + }() + + // Create tar writer + tw := tar.NewWriter(tmpFile) + + // Walk the directory tree + err = filepath.Walk(dirPath, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + // Create tar header + header, err := tar.FileInfoHeader(info, "") + if err != nil { + return fmt.Errorf("create tar header for %s: %w", path, err) + } + + // Compute relative path from the parent of dirPath + relPath, err := filepath.Rel(filepath.Dir(dirPath), path) + if err != nil { + return fmt.Errorf("compute relative path: %w", err) + } + + // Use forward slashes for tar archive paths + header.Name = filepath.ToSlash(relPath) + + // Write header + if err := tw.WriteHeader(header); err != nil { + return fmt.Errorf("write tar header: %w", err) + } + + // If it's a file, write its contents + if !info.IsDir() { + file, err := os.Open(path) + if err != nil { + return fmt.Errorf("open file %s: %w", path, err) + } + defer file.Close() + + // Copy file contents + if _, err := io.Copy(tw, file); err != nil { + return fmt.Errorf("write tar content for %s: %w", path, err) + } + } + + return nil + }) + + if err != nil { + tw.Close() + tmpFile.Close() + return "", fmt.Errorf("walk directory: %w", err) + } + + // Close tar writer + if err := tw.Close(); err != nil { + tmpFile.Close() + return "", fmt.Errorf("close tar writer: %w", err) + } + + // Close temp file + if err := tmpFile.Close(); err != nil { + return "", fmt.Errorf("close temp file: %w", err) + } + + shouldKeepTempFile = true + return tmpPath, nil +} diff --git a/pkg/distribution/packaging/safetensors.go b/pkg/distribution/packaging/safetensors.go index cf14c977f..379fb4a38 100644 --- a/pkg/distribution/packaging/safetensors.go +++ b/pkg/distribution/packaging/safetensors.go @@ -144,105 +144,3 @@ func addFileToTar(tw *tar.Writer, filePath string) error { return nil } - -// CreateDirectoryTarArchive creates a temporary tar archive containing the specified directory -// with its structure preserved. It returns the path to the temporary tar file and any error encountered. -// The caller is responsible for removing the temporary file when done. -func CreateDirectoryTarArchive(dirPath string) (string, error) { - // Verify directory exists - info, err := os.Stat(dirPath) - if err != nil { - return "", fmt.Errorf("stat directory: %w", err) - } - if !info.IsDir() { - return "", fmt.Errorf("path is not a directory: %s", dirPath) - } - - // Create temp file - tmpFile, err := os.CreateTemp("", "dir-tar-*.tar") - if err != nil { - return "", fmt.Errorf("create temp file: %w", err) - } - tmpPath := tmpFile.Name() - - // Track success to determine if we should clean up the temp file - shouldKeepTempFile := false - defer func() { - if !shouldKeepTempFile { - os.Remove(tmpPath) - } - }() - - // Create tar writer - tw := tar.NewWriter(tmpFile) - - // Walk the directory tree - err = filepath.Walk(dirPath, func(path string, info os.FileInfo, err error) error { - if err != nil { - return err - } - - // Create tar header - header, err := tar.FileInfoHeader(info, "") - if err != nil { - return fmt.Errorf("create tar header for %s: %w", path, err) - } - - // Compute relative path from the parent of dirPath - relPath, err := filepath.Rel(filepath.Dir(dirPath), path) - if err != nil { - return fmt.Errorf("compute relative path: %w", err) - } - - // Use forward slashes for tar archive paths - header.Name = filepath.ToSlash(relPath) - - // Write header - if err := tw.WriteHeader(header); err != nil { - return fmt.Errorf("write tar header: %w", err) - } - - // If it's a file, write its contents - if !info.IsDir() { - file, err := os.Open(path) - if err != nil { - return fmt.Errorf("open file %s: %w", path, err) - } - - // Copy file contents - _, copyErr := io.Copy(tw, file) - - // Close immediately to avoid file descriptor exhaustion in large directories - closeErr := file.Close() - - if copyErr != nil { - return fmt.Errorf("write tar content for %s: %w", path, copyErr) - } - if closeErr != nil { - return fmt.Errorf("close file %s: %w", path, closeErr) - } - } - - return nil - }) - - if err != nil { - tw.Close() - tmpFile.Close() - return "", fmt.Errorf("walk directory: %w", err) - } - - // Close tar writer - if err := tw.Close(); err != nil { - tmpFile.Close() - return "", fmt.Errorf("close tar writer: %w", err) - } - - // Close temp file - if err := tmpFile.Close(); err != nil { - return "", fmt.Errorf("close temp file: %w", err) - } - - shouldKeepTempFile = true - return tmpPath, nil -} From 2867491aec715750a9e78f3be40c2f52ea4db51b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20L=C3=B3pez=20Luna?= Date: Thu, 16 Oct 2025 14:27:56 +0200 Subject: [PATCH 05/16] refactor(unpack): streamline tar extraction by removing temporary file usage --- pkg/distribution/internal/bundle/unpack.go | 56 ++++++++-------------- 1 file changed, 21 insertions(+), 35 deletions(-) diff --git a/pkg/distribution/internal/bundle/unpack.go b/pkg/distribution/internal/bundle/unpack.go index 780a21ad3..d7d19bff3 100644 --- a/pkg/distribution/internal/bundle/unpack.go +++ b/pkg/distribution/internal/bundle/unpack.go @@ -263,37 +263,18 @@ func unpackDirTarArchives(bundle *Bundle, mdl types.Model) error { continue } - // Get the layer as a compressed blob - compressed, err := layer.Compressed() + // Get the layer as an uncompressed stream (decompression handled automatically) + uncompressed, err := layer.Uncompressed() if err != nil { - return fmt.Errorf("get compressed layer: %w", err) + return fmt.Errorf("get uncompressed layer: %w", err) } - // Create a temp file to store the layer - tempFile, err := os.CreateTemp("", "dir-tar-*.tar") - if err != nil { - compressed.Close() - return fmt.Errorf("create temp file: %w", err) - } - tempPath := tempFile.Name() - - // Copy the layer to the temp file - if _, err := io.Copy(tempFile, compressed); err != nil { - tempFile.Close() - compressed.Close() - return fmt.Errorf("copy layer to temp file: %w", err) - } - tempFile.Close() - compressed.Close() - - // Extract the tar archive into the model subdirectory - if err := extractTarArchive(tempPath, modelDir); err != nil { - os.Remove(tempPath) + // Stream directly to tar extraction - no temp file needed + if err := extractTarArchiveFromReader(uncompressed, modelDir); err != nil { + uncompressed.Close() return fmt.Errorf("extract directory tar archive: %w", err) } - - // Clean up temp file immediately after extraction - os.Remove(tempPath) + uncompressed.Close() } return nil @@ -333,14 +314,7 @@ func validatePathWithinDirectory(baseDir, targetPath string) error { return nil } -func extractTarArchive(archivePath, destDir string) error { - // Open the tar file - file, err := os.Open(archivePath) - if err != nil { - return fmt.Errorf("open tar archive: %w", err) - } - defer file.Close() - +func extractTarArchiveFromReader(r io.Reader, destDir string) error { // Get absolute path of destination directory for security checks absDestDir, err := filepath.Abs(destDir) if err != nil { @@ -348,7 +322,7 @@ func extractTarArchive(archivePath, destDir string) error { } // Create tar reader - tr := tar.NewReader(file) + tr := tar.NewReader(r) // Extract files for { @@ -397,6 +371,18 @@ func extractTarArchive(archivePath, destDir string) error { return nil } +func extractTarArchive(archivePath, destDir string) error { + // Open the tar file + file, err := os.Open(archivePath) + if err != nil { + return fmt.Errorf("open tar archive: %w", err) + } + defer file.Close() + + // Delegate to the streaming version + return extractTarArchiveFromReader(file, destDir) +} + // extractFile extracts a single file from the tar reader func extractFile(tr io.Reader, target string, mode os.FileMode) error { // Ensure parent directory exists From f39c0e63f29020b40cb473b234ca964e082ac7b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20L=C3=B3pez=20Luna?= Date: Thu, 16 Oct 2025 14:32:00 +0200 Subject: [PATCH 06/16] feat(packaging): skip symlinks during tar archive creation for security reasons --- pkg/distribution/packaging/dirtar.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/distribution/packaging/dirtar.go b/pkg/distribution/packaging/dirtar.go index 6896dc31d..57c735895 100644 --- a/pkg/distribution/packaging/dirtar.go +++ b/pkg/distribution/packaging/dirtar.go @@ -41,6 +41,11 @@ func CreateDirectoryTarArchive(dirPath string) (string, error) { // Walk the directory tree err = filepath.Walk(dirPath, func(path string, info os.FileInfo, err error) error { + // Skip symlinks - they're not needed for model distribution and are + // skipped during extraction for security reasons + if info.Mode()&os.ModeSymlink != 0 { + return nil + } if err != nil { return err } From 0d8e66a43ffde18dbc6b8630f303cb90b2393c6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20L=C3=B3pez=20Luna?= Date: Thu, 16 Oct 2025 14:33:13 +0200 Subject: [PATCH 07/16] make docs --- cmd/cli/docs/reference/docker_model_package.yaml | 11 +++++++++++ cmd/cli/docs/reference/model_package.md | 1 + 2 files changed, 12 insertions(+) diff --git a/cmd/cli/docs/reference/docker_model_package.yaml b/cmd/cli/docs/reference/docker_model_package.yaml index bed0cd43c..b30710331 100644 --- a/cmd/cli/docs/reference/docker_model_package.yaml +++ b/cmd/cli/docs/reference/docker_model_package.yaml @@ -28,6 +28,17 @@ options: experimentalcli: false kubernetes: false swarm: false + - option: dir-tar + value_type: stringArray + default_value: '[]' + description: | + relative path to directory to package as tar (can be specified multiple times) + deprecated: false + hidden: false + experimental: false + experimentalcli: false + kubernetes: false + swarm: false - option: gguf value_type: string description: absolute path to gguf file diff --git a/cmd/cli/docs/reference/model_package.md b/cmd/cli/docs/reference/model_package.md index 262070ac5..44e7a4e32 100644 --- a/cmd/cli/docs/reference/model_package.md +++ b/cmd/cli/docs/reference/model_package.md @@ -11,6 +11,7 @@ When packaging a Safetensors model, --safetensors-dir should point to a director |:--------------------|:--------------|:--------|:---------------------------------------------------------------------------------------| | `--chat-template` | `string` | | absolute path to chat template file (must be Jinja format) | | `--context-size` | `uint64` | `0` | context size in tokens | +| `--dir-tar` | `stringArray` | | relative path to directory to package as tar (can be specified multiple times) | | `--gguf` | `string` | | absolute path to gguf file | | `-l`, `--license` | `stringArray` | | absolute path to a license file | | `--push` | `bool` | | push to registry (if not set, the model is loaded into the Model Runner content store) | From 38faf8c2a5c23cfa3d6e30fd32e5492337f0ab73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20L=C3=B3pez=20Luna?= Date: Thu, 16 Oct 2025 15:16:32 +0200 Subject: [PATCH 08/16] refactor(package): consolidate cleanup of temporary tar files --- cmd/cli/commands/package.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/cmd/cli/commands/package.go b/cmd/cli/commands/package.go index bb2c08695..10d16dc26 100644 --- a/cmd/cli/commands/package.go +++ b/cmd/cli/commands/package.go @@ -218,6 +218,13 @@ func packageModel(cmd *cobra.Command, opts packageOptions) error { // Process directory tar archives var tempDirTarFiles []string if len(opts.dirTarPaths) > 0 { + // Schedule cleanup of temp tar files + defer func() { + for _, tempFile := range tempDirTarFiles { + os.Remove(tempFile) + } + }() + // Determine base directory for resolving relative paths var baseDir string if opts.safetensorsDir != "" { @@ -262,13 +269,6 @@ func packageModel(cmd *cobra.Command, opts packageOptions) error { return fmt.Errorf("add directory tar: %w", err) } } - - // Schedule cleanup of temp tar files after build completes - defer func() { - for _, tempFile := range tempDirTarFiles { - os.Remove(tempFile) - } - }() } if opts.push { From eea47c1dc332949967f8a9451f0df7ae3580d53c Mon Sep 17 00:00:00 2001 From: Ignasi Date: Thu, 16 Oct 2025 19:20:55 +0200 Subject: [PATCH 09/16] Update pkg/distribution/packaging/dirtar.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- pkg/distribution/packaging/dirtar.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/distribution/packaging/dirtar.go b/pkg/distribution/packaging/dirtar.go index 57c735895..9b313690e 100644 --- a/pkg/distribution/packaging/dirtar.go +++ b/pkg/distribution/packaging/dirtar.go @@ -41,14 +41,17 @@ func CreateDirectoryTarArchive(dirPath string) (string, error) { // Walk the directory tree err = filepath.Walk(dirPath, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if info == nil { + return fmt.Errorf("nil FileInfo for path: %s", path) + } // Skip symlinks - they're not needed for model distribution and are // skipped during extraction for security reasons if info.Mode()&os.ModeSymlink != 0 { return nil } - if err != nil { - return err - } // Create tar header header, err := tar.FileInfoHeader(info, "") From f9cdf1ce8da81be35664b1d154fb4019183824a2 Mon Sep 17 00:00:00 2001 From: Ignasi Date: Thu, 16 Oct 2025 19:28:31 +0200 Subject: [PATCH 10/16] Update pkg/distribution/packaging/dirtar.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- pkg/distribution/packaging/dirtar.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/distribution/packaging/dirtar.go b/pkg/distribution/packaging/dirtar.go index 9b313690e..79f6297e7 100644 --- a/pkg/distribution/packaging/dirtar.go +++ b/pkg/distribution/packaging/dirtar.go @@ -79,12 +79,16 @@ func CreateDirectoryTarArchive(dirPath string) (string, error) { if err != nil { return fmt.Errorf("open file %s: %w", path, err) } - defer file.Close() + // Copy file contents if _, err := io.Copy(tw, file); err != nil { + file.Close() return fmt.Errorf("write tar content for %s: %w", path, err) } + if err := file.Close(); err != nil { + return fmt.Errorf("close file %s: %w", path, err) + } } return nil From f5eb1670ba5dccbe69197ac2ff4e8a87640bdc18 Mon Sep 17 00:00:00 2001 From: Ignasi Date: Thu, 16 Oct 2025 19:29:29 +0200 Subject: [PATCH 11/16] Update pkg/distribution/internal/bundle/unpack.go Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com> --- pkg/distribution/internal/bundle/unpack.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/distribution/internal/bundle/unpack.go b/pkg/distribution/internal/bundle/unpack.go index d7d19bff3..2bb6548c5 100644 --- a/pkg/distribution/internal/bundle/unpack.go +++ b/pkg/distribution/internal/bundle/unpack.go @@ -255,6 +255,7 @@ func unpackDirTarArchives(bundle *Bundle, mdl types.Model) error { for _, layer := range layers { mediaType, err := layer.MediaType() if err != nil { + log.Printf("error getting media type for layer: %v", err) continue } From 03a367fd54a0b17c0ba6ad32994f4babd31abd41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20L=C3=B3pez=20Luna?= Date: Thu, 16 Oct 2025 19:32:29 +0200 Subject: [PATCH 12/16] refactor(package): remove temporary file cleanup from tar archive creation, it's already done via defer --- cmd/cli/commands/package.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/cmd/cli/commands/package.go b/cmd/cli/commands/package.go index 10d16dc26..8b6d43345 100644 --- a/cmd/cli/commands/package.go +++ b/cmd/cli/commands/package.go @@ -251,10 +251,6 @@ func packageModel(cmd *cobra.Command, opts packageOptions) error { cmd.PrintErrf("Creating tar archive for directory %q\n", relDirPath) tempTarPath, err := packaging.CreateDirectoryTarArchive(fullDirPath) if err != nil { - // Clean up any temp files created so far - for _, tempFile := range tempDirTarFiles { - os.Remove(tempFile) - } return fmt.Errorf("create tar archive for directory %q: %w", relDirPath, err) } tempDirTarFiles = append(tempDirTarFiles, tempTarPath) @@ -262,10 +258,6 @@ func packageModel(cmd *cobra.Command, opts packageOptions) error { cmd.PrintErrf("Adding directory tar archive from %q\n", relDirPath) pkg, err = pkg.WithDirTar(tempTarPath) if err != nil { - // Clean up temp files - for _, tempFile := range tempDirTarFiles { - os.Remove(tempFile) - } return fmt.Errorf("add directory tar: %w", err) } } From 66cbf1c6bff7bee2ac06fe90a14bf70ef784a942 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20L=C3=B3pez=20Luna?= Date: Thu, 16 Oct 2025 19:36:46 +0200 Subject: [PATCH 13/16] refactor(unpack): change error logging to warning for media type retrieval --- pkg/distribution/internal/bundle/unpack.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/distribution/internal/bundle/unpack.go b/pkg/distribution/internal/bundle/unpack.go index 2bb6548c5..d7b9cdea4 100644 --- a/pkg/distribution/internal/bundle/unpack.go +++ b/pkg/distribution/internal/bundle/unpack.go @@ -255,7 +255,7 @@ func unpackDirTarArchives(bundle *Bundle, mdl types.Model) error { for _, layer := range layers { mediaType, err := layer.MediaType() if err != nil { - log.Printf("error getting media type for layer: %v", err) + fmt.Printf("Warning: failed to get media type for layer: %v", err) continue } From bfa9860eb06387e9222426022aefba9bf6f68b4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20L=C3=B3pez=20Luna?= Date: Thu, 16 Oct 2025 21:32:25 +0200 Subject: [PATCH 14/16] remove unnecessary blank line before file copy operation --- pkg/distribution/packaging/dirtar.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/distribution/packaging/dirtar.go b/pkg/distribution/packaging/dirtar.go index 79f6297e7..d46a1c49d 100644 --- a/pkg/distribution/packaging/dirtar.go +++ b/pkg/distribution/packaging/dirtar.go @@ -80,7 +80,6 @@ func CreateDirectoryTarArchive(dirPath string) (string, error) { return fmt.Errorf("open file %s: %w", path, err) } - // Copy file contents if _, err := io.Copy(tw, file); err != nil { file.Close() From 5923794c3705cb7c1261d09f5dcd38969d830f56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20L=C3=B3pez=20Luna?= Date: Fri, 17 Oct 2025 09:52:32 +0200 Subject: [PATCH 15/16] add validation for relative dir-tar paths to prevent directory traversal --- cmd/cli/commands/package.go | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/cmd/cli/commands/package.go b/cmd/cli/commands/package.go index 8b6d43345..eb41b6d3c 100644 --- a/cmd/cli/commands/package.go +++ b/cmd/cli/commands/package.go @@ -106,6 +106,18 @@ func newPackagedCmd() *cobra.Command { } opts.licensePaths[i] = filepath.Clean(l) } + + // Validate dir-tar paths are relative (not absolute) + for _, dirPath := range opts.dirTarPaths { + if filepath.IsAbs(dirPath) { + return fmt.Errorf( + "dir-tar path must be relative, got absolute path: %s\n\n"+ + "See 'docker model package --help' for more information", + dirPath, + ) + } + } + return nil }, RunE: func(cmd *cobra.Command, args []string) error { @@ -235,10 +247,25 @@ func packageModel(cmd *cobra.Command, opts packageOptions) error { } for _, relDirPath := range opts.dirTarPaths { + // Reject absolute paths + if filepath.IsAbs(relDirPath) { + return fmt.Errorf("dir-tar path must be relative: %s", relDirPath) + } + // Resolve the full directory path fullDirPath := filepath.Join(baseDir, relDirPath) fullDirPath = filepath.Clean(fullDirPath) + // Verify the resolved path is within baseDir to prevent directory traversal + relPath, err := filepath.Rel(baseDir, fullDirPath) + if err != nil { + return fmt.Errorf("dir-tar path %q could not be validated: %w", relDirPath, err) + } + // Check if the relative path tries to escape the base directory + if relPath == ".." || len(relPath) >= 3 && relPath[:3] == ".."+string(filepath.Separator) { + return fmt.Errorf("dir-tar path %q escapes base directory", relDirPath) + } + // Verify the directory exists info, err := os.Stat(fullDirPath) if err != nil { From 59bb31466916640c9d44e459322dbe16562a4cd6 Mon Sep 17 00:00:00 2001 From: Ignasi Date: Fri, 17 Oct 2025 09:59:03 +0200 Subject: [PATCH 16/16] Update pkg/distribution/packaging/dirtar.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- pkg/distribution/packaging/dirtar.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/distribution/packaging/dirtar.go b/pkg/distribution/packaging/dirtar.go index d46a1c49d..9998e0374 100644 --- a/pkg/distribution/packaging/dirtar.go +++ b/pkg/distribution/packaging/dirtar.go @@ -9,7 +9,8 @@ import ( ) // CreateDirectoryTarArchive creates a temporary tar archive containing the specified directory -// with its structure preserved. It returns the path to the temporary tar file and any error encountered. +// with its structure preserved. Symlinks encountered in the directory are skipped and will not be included +// in the archive. It returns the path to the temporary tar file and any error encountered. // The caller is responsible for removing the temporary file when done. func CreateDirectoryTarArchive(dirPath string) (string, error) { // Verify directory exists