diff --git a/internal/utils/archive/archive.go b/internal/utils/archive/archive.go index 2a7853c0..f5eba646 100644 --- a/internal/utils/archive/archive.go +++ b/internal/utils/archive/archive.go @@ -374,7 +374,7 @@ func extractEntry(root *os.Root, header *tar.Header, tarReader io.Reader, cfg ex directoryMode := os.FileMode(header.Mode) & os.ModePerm //nolint:gosec // mask tar mode to permission bits - if err := root.MkdirAll(directoryName, fileperms.PublicDir); err != nil { + if err := ensureDirectory(root, directoryName); err != nil { return fmt.Errorf("creating directory %#q:\n%w", name, err) } @@ -383,7 +383,7 @@ func extractEntry(root *os.Root, header *tar.Header, tarReader io.Reader, cfg ex return nil } - if err := root.MkdirAll(filepath.Dir(name), fileperms.PublicDir); err != nil { + if err := ensureDirectory(root, filepath.Dir(name)); err != nil { return fmt.Errorf("creating parent for %#q:\n%w", name, err) } @@ -423,6 +423,45 @@ func extractEntry(root *os.Root, header *tar.Header, tarReader io.Reader, cfg ex } } +// ensureDirectory creates name and applies a deterministic mode to every +// implicit parent it materializes. MkdirAll applies the process umask, so the +// mode is restored immediately instead of being deferred with explicitly +// archived directory modes. This avoids applying a default mode through a +// symlink alias after an explicit directory entry has restored its own mode. +func ensureDirectory(root *os.Root, name string) error { + name = filepath.Clean(name) + if name == "." { + return nil + } + + var missingPaths []string + + for current := name; current != "."; current = filepath.Dir(current) { + if _, err := root.Stat(current); err == nil { + break + } else if !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("checking directory %#q:\n%w", current, err) + } + + missingPaths = append(missingPaths, current) + } + + if err := root.MkdirAll(name, fileperms.PublicDir); err != nil { + return fmt.Errorf("materializing directory %#q:\n%w", name, err) + } + + // Set parents before children. A restrictive umask can otherwise make a + // newly-created parent untraversable before its child is chmodded. + for idx := len(missingPaths) - 1; idx >= 0; idx-- { + path := missingPaths[idx] + if err := root.Chmod(path, fileperms.PublicDir); err != nil { + return fmt.Errorf("setting permissions on implicit directory %#q:\n%w", path, err) + } + } + + return nil +} + // restoreDirectoryModes applies archive directory modes after all content has // been extracted. Deepest paths are restored first so a restrictive parent mode // cannot prevent reaching an explicit child directory. @@ -479,6 +518,12 @@ func extractRegularFile(root *os.Root, header *tar.Header, src io.Reader) (err e return fmt.Errorf("writing file %#q:\n%w", name, copyErr) } + // OpenFile applies the process umask when it creates the file. Restore the + // archived permission bits explicitly so repacking is host-independent. + if chmodErr := outFile.Chmod(mode); chmodErr != nil { + return fmt.Errorf("setting permissions on file %#q:\n%w", name, chmodErr) + } + return nil } diff --git a/internal/utils/archive/archive_linux_test.go b/internal/utils/archive/archive_linux_test.go new file mode 100644 index 00000000..ba29843f --- /dev/null +++ b/internal/utils/archive/archive_linux_test.go @@ -0,0 +1,76 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//go:build linux + +package archive_test + +import ( + "archive/tar" + "os" + "path/filepath" + "syscall" + "testing" + + "github.com/microsoft/azure-linux-dev-tools/internal/utils/archive" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestExtract_PreservesModesUnderRestrictiveUmask(t *testing.T) { + tmpDir := t.TempDir() + archivePath := filepath.Join(tmpDir, "source.tar.gz") + + createTestTarGz(t, archivePath, []testTarEntry{ + {name: "implicit/file.txt", typeflag: tar.TypeReg, content: "content", mode: 0o666}, + }) + + repack := func(name string, umask int) []byte { + extractDir := filepath.Join(tmpDir, name) + repackedPath := filepath.Join(tmpDir, name+".tar.gz") + + previousUmask := syscall.Umask(umask) + defer syscall.Umask(previousUmask) + + require.NoError(t, archive.Extract(archivePath, extractDir, archive.CompressionGzip)) + require.NoError(t, archive.CreateDeterministicArchive(repackedPath, extractDir, archive.CompressionGzip)) + + directoryInfo, err := os.Stat(filepath.Join(extractDir, "implicit")) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0o755), directoryInfo.Mode().Perm()) + + fileInfo, err := os.Stat(filepath.Join(extractDir, "implicit", "file.txt")) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0o666), fileInfo.Mode().Perm()) + + data, err := os.ReadFile(repackedPath) + require.NoError(t, err) + + return data + } + + standard := repack("standard", 0o022) + restrictive := repack("restrictive", 0o077) + assert.Equal(t, standard, restrictive, "repacked archive must not depend on the process umask") +} + +func TestExtract_ImplicitDirectorySymlinkAliasDoesNotOverrideExplicitMode(t *testing.T) { + tmpDir := t.TempDir() + archivePath := filepath.Join(tmpDir, "source.tar.gz") + extractDir := filepath.Join(tmpDir, "extracted") + + createTestTarGz(t, archivePath, []testTarEntry{ + {name: "real/", typeflag: tar.TypeDir, mode: 0o700}, + {name: "x", typeflag: tar.TypeSymlink, linkname: "real"}, + {name: "x/subdir/file", typeflag: tar.TypeReg, content: "content"}, + {name: "real/subdir/", typeflag: tar.TypeDir, mode: 0o700}, + }) + + require.NoError(t, archive.Extract(archivePath, extractDir, archive.CompressionGzip)) + + for _, path := range []string{"real/subdir", "x/subdir"} { + info, err := os.Stat(filepath.Join(extractDir, path)) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0o700), info.Mode().Perm(), "directory %#q mode", path) + } +} diff --git a/internal/utils/archive/archive_test.go b/internal/utils/archive/archive_test.go index a7d2a1a3..82bd5baf 100644 --- a/internal/utils/archive/archive_test.go +++ b/internal/utils/archive/archive_test.go @@ -11,6 +11,7 @@ import ( "io" "os" "path/filepath" + "runtime" "testing" "github.com/microsoft/azure-linux-dev-tools/internal/utils/archive" @@ -177,7 +178,11 @@ func createTestTarGz(t *testing.T, path string, entries []testTarEntry) { header.Mode = 0o755 } case tar.TypeReg: - header.Mode = 0o644 + header.Mode = entry.mode + if header.Mode == 0 { + header.Mode = 0o644 + } + header.Size = int64(len(entry.content)) case tar.TypeSymlink: header.Linkname = entry.linkname @@ -250,6 +255,36 @@ func TestRoundTrip_AllCompressions(t *testing.T) { } } +func TestCreateDeterministicArchive_ZstdIndependentOfGOMAXPROCS(t *testing.T) { + tmpDir := t.TempDir() + sourceDir := filepath.Join(tmpDir, "src") + require.NoError(t, os.MkdirAll(sourceDir, 0o755)) + require.NoError(t, os.WriteFile( + filepath.Join(sourceDir, "content.bin"), + bytes.Repeat([]byte("deterministic content\n"), 64*1024), + 0o600, + )) + + create := func(name string, maxProcs int) []byte { + previousMaxProcs := runtime.GOMAXPROCS(maxProcs) + defer runtime.GOMAXPROCS(previousMaxProcs) + + archivePath := filepath.Join(tmpDir, name+".tar.zst") + require.NoError(t, archive.CreateDeterministicArchive( + archivePath, sourceDir, archive.CompressionZstd, + )) + + data, err := os.ReadFile(archivePath) + require.NoError(t, err) + + return data + } + + singleCPU := create("single-cpu", 1) + multipleCPUs := create("multiple-cpus", 4) + assert.Equal(t, singleCPU, multipleCPUs, "zstd output must not depend on GOMAXPROCS") +} + func TestExtractAndRepack_PreservesExplicitDirectoryPermissions(t *testing.T) { tmpDir := t.TempDir() extractDir := filepath.Join(tmpDir, "extracted")