Skip to content

Commit bdee3df

Browse files
committed
fix: improve error reporting for invalid filepath annotations during sanitization
1 parent 461082f commit bdee3df

2 files changed

Lines changed: 47 additions & 1 deletion

File tree

pkg/distribution/internal/bundle/unpack.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -722,7 +722,13 @@ func UnpackFromLayers(dir string, model types.ModelArtifact) (*Bundle, error) {
722722
sanitizedPath := sanitizeRelativePath(relPath)
723723
// Re-validate the sanitized path to ensure it's safe.
724724
if err2 := validatePathWithinDirectory(modelDir, sanitizedPath); err2 != nil {
725-
return nil, fmt.Errorf("invalid filepath annotation %q could not be sanitized: %w", relPath, err)
725+
return nil, fmt.Errorf(
726+
"invalid filepath annotation %q (sanitized as %q): original error: %w, sanitized error: %w",
727+
relPath,
728+
sanitizedPath,
729+
err,
730+
err2,
731+
)
726732
}
727733
relPath = sanitizedPath
728734
}

pkg/distribution/internal/bundle/unpack_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -541,6 +541,46 @@ func TestUnpackFromLayers_DuplicateRawAnnotationAllowed(t *testing.T) {
541541
}
542542
}
543543

544+
func TestUnpackFromLayers_PathSanitizationRejectsCollapsedPath(t *testing.T) {
545+
// Build a ModelPack artifact whose annotation collapses entirely during
546+
// sanitization. This must fail before any file is written.
547+
artifact := testutil.NewModelPackArtifact(
548+
t,
549+
modelpack.Model{
550+
Config: modelpack.ModelConfig{Format: string(types.FormatGGUF)},
551+
},
552+
testutil.LayerSpec{
553+
Path: filepath.Join("..", "..", "assets", "dummy.gguf"),
554+
RelativePath: "../../..",
555+
MediaType: oci.MediaType(modelpack.MediaTypeWeightGGUF),
556+
},
557+
)
558+
559+
bundleRoot := t.TempDir()
560+
_, err := UnpackFromLayers(bundleRoot, artifact)
561+
if err == nil {
562+
t.Fatal("Expected sanitization error, got nil")
563+
}
564+
if !strings.Contains(err.Error(), `invalid filepath annotation "../../.."`) {
565+
t.Fatalf("Expected error to mention original annotation, got: %v", err)
566+
}
567+
if !strings.Contains(err.Error(), `sanitized as ""`) {
568+
t.Fatalf("Expected error to mention sanitized path, got: %v", err)
569+
}
570+
if !strings.Contains(err.Error(), "empty path is not allowed") {
571+
t.Fatalf("Expected error to mention sanitized validation failure, got: %v", err)
572+
}
573+
574+
modelDir := filepath.Join(bundleRoot, ModelSubdir)
575+
entries, readErr := os.ReadDir(modelDir)
576+
if readErr != nil {
577+
t.Fatalf("Expected model directory to exist, got: %v", readErr)
578+
}
579+
if len(entries) != 0 {
580+
t.Fatalf("Expected no files to be written for rejected annotation, got %d entries", len(entries))
581+
}
582+
}
583+
544584
func TestValidatePathWithinDirectory_RealFilesystem(t *testing.T) {
545585
// Create a temporary directory structure
546586
baseDir := t.TempDir()

0 commit comments

Comments
 (0)