simplify MkdirTmp usage

Signed-off-by: egibs <20933572+egibs@users.noreply.github.com>

Author: egibs

pkg/archive/archive.go

@@ -164,13 +164,13 @@ func extractNestedArchive(ctx context.Context, c malcontent.Config, d string, f
 	// Some packages may have archives and files with colliding names
 	// e.g., demo_page.css and demo_page.css.gz
 	// the former is the uncompressed version of the latter
-	// if we encounter this, replace the name with something that won't collide
+	// if we encounter this, use os.MkdirTemp to create a unique directory
 	if _, err := os.Stat(archivePath); err == nil {
 		logger.Debugf("duplicate file name already exists, modifying directory name for %s", archivePath)
-		var err error
-		archivePath, err = os.MkdirTemp(d, strings.TrimSuffix(f, programkind.GetExt(f))+"_*")
-		if err != nil {
-			return fmt.Errorf("failed to create unique extraction directory: %w", err)
+		var mkErr error
+		archivePath, mkErr = os.MkdirTemp(filepath.Dir(archivePath), filepath.Base(archivePath)+"_*")
+		if mkErr != nil {
+			return fmt.Errorf("failed to create unique extraction directory: %w", mkErr)
 		}
 	} else if err := os.MkdirAll(archivePath, 0o700); err != nil {
 		return fmt.Errorf("failed to create extraction directory: %w", err)

pkg/archive/symlink_test.go

@@ -7,7 +7,11 @@ import (
 	"context"
 	"os"
 	"path/filepath"
+	"sync"
 	"testing"
+
+	"github.com/chainguard-dev/clog"
+	"github.com/chainguard-dev/malcontent/pkg/malcontent"
 )
 
 func TestSymlinkExtraction(t *testing.T) {
@@ -109,6 +113,88 @@ func TestValidateResolvedPath(t *testing.T) {
 	}
 }
 
+// TestExtractNestedArchiveWithSubdirectory verifies that extractNestedArchive
+// handles archives located in subdirectories (where the relative path contains
+// path separators). This is a regression test for a bug where os.MkdirTemp
+// was called with a pattern containing path separators, which is not allowed.
+func TestExtractNestedArchiveWithSubdirectory(t *testing.T) {
+	t.Parallel()
+
+	tmpDir, err := os.MkdirTemp("", "nested-archive-test-*")
+	if err != nil {
+		t.Fatalf("failed to create temp dir: %v", err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	// Create a nested archive inside a subdirectory, simulating what happens
+	// when an archive contains another archive at a path like "subdir/inner.tar.gz"
+	subDir := filepath.Join(tmpDir, "subdir")
+	if err := os.MkdirAll(subDir, 0o700); err != nil {
+		t.Fatalf("failed to create subdir: %v", err)
+	}
+
+	// Copy a real .gz test file into the subdirectory
+	srcData, err := os.ReadFile("../../pkg/action/testdata/apko.gz")
+	if err != nil {
+		t.Fatalf("failed to read test archive: %v", err)
+	}
+	nestedArchive := filepath.Join(subDir, "apko.gz")
+	if err := os.WriteFile(nestedArchive, srcData, 0o600); err != nil {
+		t.Fatalf("failed to write nested archive: %v", err)
+	}
+
+	ctx := context.Background()
+	logger := clog.FromContext(ctx)
+	cfg := malcontent.Config{}
+	var extracted sync.Map
+
+	// This is the call that previously failed with "pattern contains path separator"
+	err = extractNestedArchive(ctx, cfg, tmpDir, "subdir/apko.gz", &extracted, logger, 1)
+	if err != nil {
+		t.Fatalf("extractNestedArchive failed: %v", err)
+	}
+}
+
+// TestExtractNestedArchiveCollision verifies that extractNestedArchive handles
+// name collisions by falling back to os.MkdirTemp when the deterministic path
+// already exists.
+func TestExtractNestedArchiveCollision(t *testing.T) {
+	t.Parallel()
+
+	tmpDir, err := os.MkdirTemp("", "nested-collision-test-*")
+	if err != nil {
+		t.Fatalf("failed to create temp dir: %v", err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	// Create a file that will collide with the extraction directory name.
+	// When extracting "apko.gz", the extraction dir would be "apko" — create
+	// that as a file first to force the collision path.
+	collisionPath := filepath.Join(tmpDir, "apko")
+	if err := os.WriteFile(collisionPath, []byte("existing"), 0o600); err != nil {
+		t.Fatalf("failed to create collision file: %v", err)
+	}
+
+	srcData, err := os.ReadFile("../../pkg/action/testdata/apko.gz")
+	if err != nil {
+		t.Fatalf("failed to read test archive: %v", err)
+	}
+	archivePath := filepath.Join(tmpDir, "apko.gz")
+	if err := os.WriteFile(archivePath, srcData, 0o600); err != nil {
+		t.Fatalf("failed to write archive: %v", err)
+	}
+
+	ctx := context.Background()
+	logger := clog.FromContext(ctx)
+	cfg := malcontent.Config{}
+	var extracted sync.Map
+
+	err = extractNestedArchive(ctx, cfg, tmpDir, "apko.gz", &extracted, logger, 1)
+	if err != nil {
+		t.Fatalf("extractNestedArchive with collision failed: %v", err)
+	}
+}
+
 func TestHandleSymlink(t *testing.T) {
 	t.Parallel()
 	tmpDir, err := os.MkdirTemp("", "symlink-test-*")

pkg/archive/zip.go

@@ -114,11 +114,12 @@ func extractFile(ctx context.Context, zf *zip.File, destDir string, logger *clog
 	// this case insensitivity will break scans, so rename files that collide with existing directories
 	if runtime.GOOS == "darwin" {
 		if _, err := os.Stat(filepath.Join(destDir, zf.Name)); err == nil {
-			tmpFile, err := os.CreateTemp(destDir, filepath.Base(zf.Name)+"_*")
-			if err == nil {
-				zf.Name = filepath.Base(tmpFile.Name())
-				tmpFile.Close()
-				os.Remove(tmpFile.Name())
+			uniqueDir, mkErr := os.MkdirTemp(filepath.Join(destDir, filepath.Dir(zf.Name)), filepath.Base(zf.Name)+"_*")
+			if mkErr == nil {
+				rel, relErr := filepath.Rel(destDir, uniqueDir)
+				if relErr == nil {
+					zf.Name = rel
+				}
 			}
 		}
 	}