From 7c6eb08cf5b524fd99e757dc13eafbe49e6d8ebf Mon Sep 17 00:00:00 2001 From: Antonio Salinas Date: Mon, 27 Jul 2026 22:35:31 +0000 Subject: [PATCH 1/3] fix: ensure archive dir is copied deterministically --- internal/utils/archive/archive.go | 43 ++++++++++++++- internal/utils/archive/archive_linux_test.go | 55 ++++++++++++++++++++ internal/utils/archive/archive_test.go | 37 ++++++++++++- 3 files changed, 132 insertions(+), 3 deletions(-) create mode 100644 internal/utils/archive/archive_linux_test.go diff --git a/internal/utils/archive/archive.go b/internal/utils/archive/archive.go index 2a7853c0..47b7d7d5 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, cfg.directoryModes); 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), cfg.directoryModes); err != nil { return fmt.Errorf("creating parent for %#q:\n%w", name, err) } @@ -423,6 +423,39 @@ func extractEntry(root *os.Root, header *tar.Header, tarReader io.Reader, cfg ex } } +// ensureDirectory creates name and records a deterministic mode for every +// implicit parent it materializes. MkdirAll applies the process umask, so the +// recorded modes must be restored after extraction just like explicit tar +// directory entries. A later explicit entry overwrites the default mode. +func ensureDirectory(root *os.Root, name string, directoryModes map[string]os.FileMode) 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) + } + + for _, path := range missingPaths { + directoryModes[path] = fileperms.PublicDir + } + + 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 +512,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..c3db3d22 --- /dev/null +++ b/internal/utils/archive/archive_linux_test.go @@ -0,0 +1,55 @@ +// 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") +} diff --git a/internal/utils/archive/archive_test.go b/internal/utils/archive/archive_test.go index a7d2a1a3..7bc294bc 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), + 0o644, + )) + + 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") From c090df8cf320dee90c0826b861e66d0cc4604dad Mon Sep 17 00:00:00 2001 From: Antonio Salinas Date: Tue, 28 Jul 2026 17:03:46 +0000 Subject: [PATCH 2/3] fixed lint error --- internal/utils/archive/archive_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/utils/archive/archive_test.go b/internal/utils/archive/archive_test.go index 7bc294bc..82bd5baf 100644 --- a/internal/utils/archive/archive_test.go +++ b/internal/utils/archive/archive_test.go @@ -262,7 +262,7 @@ func TestCreateDeterministicArchive_ZstdIndependentOfGOMAXPROCS(t *testing.T) { require.NoError(t, os.WriteFile( filepath.Join(sourceDir, "content.bin"), bytes.Repeat([]byte("deterministic content\n"), 64*1024), - 0o644, + 0o600, )) create := func(name string, maxProcs int) []byte { From 486a1861b88078d9ac9d77654719d421b7b09bbc Mon Sep 17 00:00:00 2001 From: Antonio Salinas Date: Tue, 28 Jul 2026 20:36:53 +0000 Subject: [PATCH 3/3] fixed symlink thingy --- internal/utils/archive/archive.go | 22 +++++++++++++------- internal/utils/archive/archive_linux_test.go | 21 +++++++++++++++++++ 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/internal/utils/archive/archive.go b/internal/utils/archive/archive.go index 47b7d7d5..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 := ensureDirectory(root, directoryName, cfg.directoryModes); 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 := ensureDirectory(root, filepath.Dir(name), cfg.directoryModes); err != nil { + if err := ensureDirectory(root, filepath.Dir(name)); err != nil { return fmt.Errorf("creating parent for %#q:\n%w", name, err) } @@ -423,11 +423,12 @@ func extractEntry(root *os.Root, header *tar.Header, tarReader io.Reader, cfg ex } } -// ensureDirectory creates name and records a deterministic mode for every +// ensureDirectory creates name and applies a deterministic mode to every // implicit parent it materializes. MkdirAll applies the process umask, so the -// recorded modes must be restored after extraction just like explicit tar -// directory entries. A later explicit entry overwrites the default mode. -func ensureDirectory(root *os.Root, name string, directoryModes map[string]os.FileMode) error { +// 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 @@ -449,8 +450,13 @@ func ensureDirectory(root *os.Root, name string, directoryModes map[string]os.Fi return fmt.Errorf("materializing directory %#q:\n%w", name, err) } - for _, path := range missingPaths { - directoryModes[path] = fileperms.PublicDir + // 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 diff --git a/internal/utils/archive/archive_linux_test.go b/internal/utils/archive/archive_linux_test.go index c3db3d22..ba29843f 100644 --- a/internal/utils/archive/archive_linux_test.go +++ b/internal/utils/archive/archive_linux_test.go @@ -53,3 +53,24 @@ func TestExtract_PreservesModesUnderRestrictiveUmask(t *testing.T) { 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) + } +}