diff --git a/docs/user/reference/config/overlays.md b/docs/user/reference/config/overlays.md index 8db2ea77..12befac0 100644 --- a/docs/user/reference/config/overlays.md +++ b/docs/user/reference/config/overlays.md @@ -47,6 +47,32 @@ successfully makes a replacement to at least one matching file. | `file-remove` | Removes a file | `file` | Glob pattern for files to remove | | `file-rename` | Renames a file within the same directory | `file`, `replacement` | Name of file to rename | + > **Tip:** `file-remove` and `file-search-replace` can also operate inside a source archive by setting the `archive` field — see [Archive Overlays](#archive-overlays). + +### Archive Overlays + +A `file-remove` or `file-search-replace` overlay can modify files **inside** a source archive +instead of loose files in the sources tree by setting the `archive` field to the archive name +and the `file` field to a glob matched against the extracted archive contents. The archive is extracted, +the matching files are modified with the same machinery as loose-file overlays, and the archive is +repacked with its original compression format. + +``` +archive = "pkg-1.0.tar.gz" +file = "vendor/**" # loose files in the sources tree (no archive field) +file = "old.tar.gz" # removes the archive file itself (bare name, no archive field) +``` + +> **Note:** Archive overlays are batched per archive — all overlays targeting the same archive +> share a single extract/modify/repack cycle — and the `sources` file is rehashed afterward to +> reflect the repacked archive. They are processed independently of spec and loose-file overlays. + +> **Extraction root:** The inner path is interpreted relative to the archive's extraction root: if the archive unpacks to a single top-level directory (the conventional `%{name}-%{version}` layout) that directory is the root; otherwise the archive root is used. + +> **Supported entry types:** Only regular files, directories, and symlinks are supported inside an archive overlay's target. If the archive contains an entry that cannot be repacked safely (a hardlink, device node, FIFO, etc.), the overlay fails with an error rather than silently dropping the entry from the repacked archive. + +| `file-remove` (archive-scoped) | Removes file(s) matching a glob pattern from inside an archive | `archive`, `file` | + | `file-search-replace` (archive-scoped) | Regex-based search and replace on file(s) inside an archive | `archive`, `file`, `regex` | ## Field Reference | Field | TOML Key | Description | Used By | @@ -60,7 +86,8 @@ successfully makes a replacement to at least one matching file. | Regex | `regex` | Regular expression pattern to match | `spec-search-replace`, `file-search-replace` | | Replacement | `replacement` | Literal replacement text; capture group references like `$1` are **not** expanded. Omit or leave empty to delete matched text. | `spec-search-replace`, `file-search-replace`, `file-rename` | | Lines | `lines` | Array of text lines to insert | `spec-prepend-lines`, `spec-append-lines`, `file-prepend-lines` | -| File | `file` | The name of the non-spec file to modify or add | `file-prepend-lines`, `file-search-replace`, `file-add`, `file-remove`, `file-rename`, `patch-add` (optional), `patch-remove` | +| File | `file` | The name of the non-spec file to modify or add, or a glob pattern. When combined with the `archive` field, the glob is matched against files inside that source archive. | `file-prepend-lines`, `file-search-replace`, `file-add`, `file-remove`, `file-rename`, `patch-add` (optional), `patch-remove` | +| Archive | `archive` | The source archive to extract, modify, and repack (e.g. `pkg-1.0.tar.gz`). When set, `file` is a glob matched relative to the archive's extraction root. | `file-remove`, `file-search-replace` (optional) | | Source | `source` | Path to source file for `file-add` and `patch-add`; relative paths are relative to the config file that defines the overlay (the overlay file if loaded via [`overlay-files`](#per-file-overlay-format), otherwise the component config) | `file-add`, `patch-add` | | Metadata | `metadata` | Documentation table describing intent and provenance — see [Overlay Metadata](#overlay-metadata). Not allowed inside an overlay file loaded via `overlay-files` (the file-level `[metadata]` block applies to every overlay in the file). | All (optional) | @@ -448,6 +475,37 @@ description = "Remove CVE patches that are now upstream" > `PatchN` tags. Macro-based tag numbering (e.g., `Patch%{n}`) is not expanded and may > conflict with auto-assigned numbers. +### Removing a File from an Archive + +Set `archive` to the archive name and `file` to a glob to delete files matching the pattern from +inside the source archive. The archive is extracted, matching files are removed, and the archive is +repacked. + +```toml +[[components.mypackage.overlays]] +type = "file-remove" +archive = "mypackage-1.0.tar.gz" +file = "vendor/**" +description = "Remove all bundled vendor files" +``` + +> **Tip:** Without the `archive` field, the same `file-remove` overlay removes a loose file +> from the sources tree instead. + +### Search and Replace Inside an Archive + +Set `archive` to the archive name to rewrite content inside an archive: + +```toml +[[components.mypackage.overlays]] +type = "file-search-replace" +archive = "mypackage-1.0.tar.xz" +file = "configure.ac" +regex = "AC_CHECK_LIB\\(old_lib" +replacement = "AC_CHECK_LIB(new_lib" +description = "Update library reference in configure script" +``` + ### Removing a Section The `spec-remove-section` overlay removes an entire section from the spec, including its diff --git a/internal/app/azldev/cmds/component/preparesources.go b/internal/app/azldev/cmds/component/preparesources.go index 2f0ffcaa..647d25bf 100644 --- a/internal/app/azldev/cmds/component/preparesources.go +++ b/internal/app/azldev/cmds/component/preparesources.go @@ -138,13 +138,7 @@ func PrepareComponentSources(env *azldev.Env, options *PrepareSourcesOptions) er ) } - if options.AllowNoHashes { - preparerOpts = append(preparerOpts, sources.WithAllowNoHashes()) - } - - if options.SkipSources { - preparerOpts = append(preparerOpts, sources.WithSkipLookaside()) - } + preparerOpts = appendPrepareSourcesOptions(preparerOpts, options) preparer, err := sources.NewPreparer(sourceManager, env.FS(), env, env, preparerOpts...) if err != nil { @@ -194,3 +188,20 @@ func CheckOutputDir(env *azldev.Env, options *PrepareSourcesOptions) error { "use --force to delete and recreate it", options.OutputDir) } + +// appendPrepareSourcesOptions appends conditional preparer options that control +// hashing and lookaside behavior. +func appendPrepareSourcesOptions( + opts []sources.PreparerOption, + options *PrepareSourcesOptions, +) []sources.PreparerOption { + if options.AllowNoHashes { + opts = append(opts, sources.WithAllowNoHashes()) + } + + if options.SkipSources { + opts = append(opts, sources.WithSkipLookaside()) + } + + return opts +} diff --git a/internal/app/azldev/core/sources/archiveoverlays.go b/internal/app/azldev/core/sources/archiveoverlays.go new file mode 100644 index 00000000..31f5e851 --- /dev/null +++ b/internal/app/azldev/core/sources/archiveoverlays.go @@ -0,0 +1,257 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package sources + +import ( + "fmt" + "log/slog" + "os" + "path/filepath" + + "github.com/microsoft/azure-linux-dev-tools/internal/global/opctx" + "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/archive" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/rootfs" +) + +// applyArchiveOverlays groups archive overlays by target archive and processes +// them in order. Multiple overlays targeting the same archive are batched into +// a single extract/modify/repack cycle. File removals inside the archive reuse +// the same machinery as loose-file overlays ([applyNonSpecOverlay]). +// +// It returns the names of the archives that were actually repacked. In dry-run +// mode no archive is repacked, so the returned slice is empty even when archive +// overlays were present. +func applyArchiveOverlays( + dryRunnable opctx.DryRunnable, + eventListener opctx.EventListener, + sourcesDirPath string, + overlays []projectconfig.ComponentOverlay, +) ([]string, error) { + groups := groupOverlaysByArchive(overlays) + + if len(groups) == 0 { + return nil, nil + } + + operationCount := 0 + for _, group := range groups { + operationCount += len(group.overlays) + } + + event := eventListener.StartEvent("Applying archive overlays", + "archives", len(groups), + "operations", operationCount, + ) + defer event.End() + + var repacked []string + + for _, group := range groups { + didRepack, err := processArchive(dryRunnable, sourcesDirPath, group) + if err != nil { + return nil, fmt.Errorf("archive overlay failed for %#q:\n%w", group.archive, err) + } + + if didRepack { + repacked = append(repacked, group.archive) + } + } + + return repacked, nil +} + +// archiveGroup holds overlays targeting the same archive, preserving order. +type archiveGroup struct { + archive string + overlays []projectconfig.ComponentOverlay +} + +// groupOverlaysByArchive groups archive overlays by [projectconfig.ComponentOverlay.Archive], +// preserving insertion order within each group and across groups. +// Non-archive overlays are silently skipped. +func groupOverlaysByArchive(overlays []projectconfig.ComponentOverlay) []archiveGroup { + orderMap := make(map[string]int) + + var groups []archiveGroup + + for _, overlay := range overlays { + if !overlay.ModifiesArchive() { + continue + } + + archiveName := overlay.Archive + + idx, exists := orderMap[archiveName] + if !exists { + idx = len(groups) + orderMap[archiveName] = idx + + groups = append(groups, archiveGroup{archive: archiveName}) + } + + groups[idx].overlays = append(groups[idx].overlays, overlay) + } + + return groups +} + +// processArchive extracts an archive to a temp directory, applies all overlays, +// and deterministically repacks it with the original compression, atomically +// replacing the original via a temp file + rename. It returns true when the +// archive was repacked. In dry-run mode it returns (false, nil) without touching +// the archive on disk. +func processArchive( + dryRunnable opctx.DryRunnable, + sourcesDirPath string, + group archiveGroup, +) (repacked bool, err error) { + archivePath := filepath.Join(sourcesDirPath, group.archive) + + if dryRunnable.DryRun() { + slog.Info("Dry run; would apply archive overlays", + "archive", group.archive, "operations", len(group.overlays)) + + return false, nil + } + + // The [archive] package operates exclusively through OS primitives ([os.Root], + // os.*), so extraction must use a genuine on-disk path regardless of the injected + // FS implementation (which may be in-memory or otherwise non-OS-backed). + workDir, err := os.MkdirTemp(sourcesDirPath, ".archive-overlay-") + if err != nil { + return false, fmt.Errorf("creating temp directory:\n%w", err) + } + + defer func() { + if removeErr := os.RemoveAll(workDir); removeErr != nil { + slog.Warn("Failed to clean up archive work directory", "error", removeErr) + } + }() + + // Extract the archive; compression is inferred from the filename extension. + // Fail on entry types we can't repack (hardlinks, devices, ...) so they + // aren't silently dropped from the repacked archive. + if err := archive.ExtractAuto(archivePath, workDir, archive.WithErrorOnUnsupportedEntry()); err != nil { + return false, fmt.Errorf("extracting archive:\n%w", err) + } + + // Determine the root of the extracted content. Most source archives unpack to + // a single top-level directory (e.g., "pkg-1.0/"), which is used as the root. + extractRoot, err := resolveExtractRoot(workDir) + if err != nil { + return false, fmt.Errorf("resolving extract root:\n%w", err) + } + + // Confine an OS-backed FS to the extract root so file overlays reuse the same + // machinery as loose-file overlays. + extractFS, err := rootfs.New(extractRoot) + if err != nil { + return false, fmt.Errorf("confining FS to extract root:\n%w", err) + } + + defer func() { + if closeErr := extractFS.Close(); closeErr != nil { + slog.Warn("Failed to close extract-root FS", "error", closeErr) + } + }() + + // Apply each overlay in order. Archive overlays (file-remove / file-search-replace, + // see [projectconfig.ComponentOverlay.ModifiesArchive]) operate solely on the + // destination tree, so extractFS is passed as both source and destination FS. + for _, overlay := range group.overlays { + if err := applyNonSpecOverlay(dryRunnable, extractFS, extractFS, overlay); err != nil { + return false, fmt.Errorf("applying %#q operation:\n%w", overlay.Type, err) + } + } + + // Deterministically repack the archive, reusing the original compression, and + // atomically replace the original (see [repackArchiveAtomic]). + if err := repackArchiveAtomic(archivePath, group.archive, workDir); err != nil { + return false, err + } + + slog.Info("Archive overlay applied", "archive", group.archive) + + return true, nil +} + +// repackArchiveAtomic deterministically repacks workDir into an archive that +// replaces archivePath, reusing the compression inferred from archiveName. +// +// To avoid corrupting the fetched source archive on a mid-write failure (disk +// full, permission error, etc.), it repacks to a temp file in the same directory +// and atomically renames it over the original only on success. Repacking directly +// over archivePath would truncate it first, leaving the workspace unrecoverable +// without refetching if the repack then failed. +func repackArchiveAtomic(archivePath, archiveName, workDir string) (err error) { + // Fall back to the extension only if the archive is empty (nothing to sniff). + extComp, err := archive.DetectCompression(archiveName) + if err != nil { + return fmt.Errorf("detecting compression for %#q:\n%w", archiveName, err) + } + + comp, err := archive.SniffCompressionFromFile(archivePath, extComp) + if err != nil { + return fmt.Errorf("sniffing compression for %#q:\n%w", archiveName, err) + } + + tmpFile, err := os.CreateTemp(filepath.Dir(archivePath), "."+filepath.Base(archiveName)+".repack-*") + if err != nil { + return fmt.Errorf("creating temp archive:\n%w", err) + } + + tmpPath := tmpFile.Name() + + // Close the handle immediately; CreateDeterministicArchive reopens the path. + if closeErr := tmpFile.Close(); closeErr != nil { + _ = os.Remove(tmpPath) + + return fmt.Errorf("closing temp archive %#q:\n%w", tmpPath, closeErr) + } + + if removeErr := os.Remove(tmpPath); removeErr != nil && !os.IsNotExist(removeErr) { + return fmt.Errorf("removing temp archive placeholder %#q:\n%w", tmpPath, removeErr) + } + + // Clean up the temp file unless it was successfully renamed over the original. + repackedOK := false + + defer func() { + if !repackedOK { + if removeErr := os.Remove(tmpPath); removeErr != nil && !os.IsNotExist(removeErr) { + slog.Warn("Failed to clean up temp archive", "path", tmpPath, "error", removeErr) + } + } + }() + + if err := archive.CreateDeterministicArchive(tmpPath, workDir, comp); err != nil { + return fmt.Errorf("repacking archive:\n%w", err) + } + + if err := os.Rename(tmpPath, archivePath); err != nil { + return fmt.Errorf("replacing archive %#q with repacked archive:\n%w", archivePath, err) + } + + repackedOK = true + + return nil +} + +// resolveExtractRoot returns the effective root of an extracted archive. If workDir +// contains exactly one entry and that entry is a directory (the common case for +// source archives like "pkg-1.0/"), that subdirectory is returned; otherwise workDir +// itself is returned. +func resolveExtractRoot(workDir string) (string, error) { + entries, err := os.ReadDir(workDir) + if err != nil { + return "", fmt.Errorf("reading extracted directory:\n%w", err) + } + + if len(entries) == 1 && entries[0].IsDir() { + return filepath.Join(workDir, entries[0].Name()), nil + } + + return workDir, nil +} diff --git a/internal/app/azldev/core/sources/archiveoverlays_internal_test.go b/internal/app/azldev/core/sources/archiveoverlays_internal_test.go new file mode 100644 index 00000000..6ad9ea7d --- /dev/null +++ b/internal/app/azldev/core/sources/archiveoverlays_internal_test.go @@ -0,0 +1,558 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package sources + +import ( + "archive/tar" + "compress/gzip" + "os" + "path/filepath" + "sort" + "testing" + + "github.com/microsoft/azure-linux-dev-tools/internal/global/testctx" + "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/archive" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileperms" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGroupOverlaysByArchive(t *testing.T) { + t.Run("groups overlays by archive name preserving order", func(t *testing.T) { + overlays := []projectconfig.ComponentOverlay{ + { + Type: projectconfig.ComponentOverlayRemoveFile, + Archive: "pkg-1.0.tar.gz", + Filename: "unwanted.conf", + }, + { + Type: projectconfig.ComponentOverlayRemoveFile, + Archive: "pkg-1.0.tar.gz", + Filename: "config.h", + }, + { + Type: projectconfig.ComponentOverlayRemoveFile, + Archive: "other-2.0.tar.xz", + Filename: "docs/*.md", + }, + } + + groups := groupOverlaysByArchive(overlays) + + require.Len(t, groups, 2) + + assert.Equal(t, "pkg-1.0.tar.gz", groups[0].archive) + require.Len(t, groups[0].overlays, 2) + assert.Equal(t, "unwanted.conf", groups[0].overlays[0].Filename) + assert.Equal(t, "config.h", groups[0].overlays[1].Filename) + + assert.Equal(t, "other-2.0.tar.xz", groups[1].archive) + require.Len(t, groups[1].overlays, 1) + assert.Equal(t, "docs/*.md", groups[1].overlays[0].Filename) + }) + + t.Run("skips overlays that are not archive-scoped", func(t *testing.T) { + overlays := []projectconfig.ComponentOverlay{ + {Type: projectconfig.ComponentOverlaySetSpecTag, Tag: "Version", Value: "1.0"}, + {Type: projectconfig.ComponentOverlayRemoveFile, Archive: "pkg.tar.gz", Filename: "f"}, + // Plain (non-archive) file overlay: no archive field, so it must be skipped. + {Type: projectconfig.ComponentOverlayRemoveFile, Filename: "loose.txt"}, + // Bare archive name with no archive field: a loose removal of the archive itself. + {Type: projectconfig.ComponentOverlayRemoveFile, Filename: "drop-me.tar.gz"}, + {Type: projectconfig.ComponentOverlayAddFile, Filename: "new.txt", Source: "src"}, + } + + groups := groupOverlaysByArchive(overlays) + + require.Len(t, groups, 1) + assert.Equal(t, "pkg.tar.gz", groups[0].archive) + require.Len(t, groups[0].overlays, 1) + assert.Equal(t, "f", groups[0].overlays[0].Filename) + }) +} + +func TestResolveExtractRoot(t *testing.T) { + t.Run("infers single top-level directory", func(t *testing.T) { + workDir := t.TempDir() + require.NoError(t, os.MkdirAll(workDir+"/pkg-1.0", 0o755)) + + root, err := resolveExtractRoot(workDir) + require.NoError(t, err) + assert.Equal(t, workDir+"/pkg-1.0", root) + }) + + t.Run("falls back to workDir for multiple top-level entries", func(t *testing.T) { + workDir := t.TempDir() + require.NoError(t, os.MkdirAll(workDir+"/dirA", 0o755)) + require.NoError(t, os.WriteFile(workDir+"/loose.txt", nil, fileperms.PrivateFile)) + + root, err := resolveExtractRoot(workDir) + require.NoError(t, err) + assert.Equal(t, workDir, root) + }) +} + +// TestProcessArchive_DryRunDoesNotModifyArchive verifies that, in dry-run mode, +// processArchive skips the extract/repack cycle entirely and leaves the original +// archive on disk byte-for-byte unchanged (repacking would otherwise rewrite it). +func TestProcessArchive_DryRunDoesNotModifyArchive(t *testing.T) { + ctx := testctx.NewCtx() + ctx.DryRunValue = true + + sourcesDir := t.TempDir() + + const archiveName = "pkg-1.0.tar.gz" + + archivePath := sourcesDir + "/" + archiveName + + // Content need not be a valid archive: dry-run returns before extraction, and + // the test only asserts the bytes are untouched. + original := []byte("original archive bytes") + require.NoError(t, os.WriteFile(archivePath, original, fileperms.PrivateFile)) + + group := archiveGroup{ + archive: archiveName, + overlays: []projectconfig.ComponentOverlay{ + {Type: projectconfig.ComponentOverlayRemoveFile, Filename: "remove.conf"}, + }, + } + + repacked, err := processArchive(ctx, sourcesDir, group) + require.NoError(t, err) + assert.False(t, repacked, "dry-run must report that no archive was repacked") + + after, err := os.ReadFile(archivePath) + require.NoError(t, err) + assert.Equal(t, original, after, "dry-run must not modify the archive on disk") +} + +// stageFiles writes the given slash-separated relative path -> content map under +// root, creating parent directories as needed. +func stageFiles(t *testing.T, root string, files map[string]string) { + t.Helper() + + for rel, content := range files { + full := filepath.Join(root, filepath.FromSlash(rel)) + require.NoError(t, os.MkdirAll(filepath.Dir(full), fileperms.PublicDir)) + require.NoError(t, os.WriteFile(full, []byte(content), fileperms.PrivateFile)) + } +} + +// listRegularFiles returns the sorted, slash-separated relative paths of all +// regular files under root (directories are ignored). +func listRegularFiles(t *testing.T, root string) []string { + t.Helper() + + var files []string + + require.NoError(t, filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error { + if err != nil { + return err + } + + if d.IsDir() { + return nil + } + + rel, relErr := filepath.Rel(root, path) + if relErr != nil { + return relErr + } + + files = append(files, filepath.ToSlash(rel)) + + return nil + })) + + sort.Strings(files) + + return files +} + +// extractedRegularFiles extracts archivePath into a fresh temp dir and returns +// the sorted relative paths of the regular files it contains. +func extractedRegularFiles(t *testing.T, archivePath string) []string { + t.Helper() + + out := t.TempDir() + require.NoError(t, archive.ExtractAuto(archivePath, out)) + + return listRegularFiles(t, out) +} + +// rawTarEntry describes a single entry for [buildTarGz], which writes a tarball +// directly so that entry types [archive.CreateDeterministicArchive] never emits +// (e.g. hardlinks) can be constructed for tests. +type rawTarEntry struct { + name string + typeflag byte + content string + linkname string +} + +// buildTarGz writes a gzip-compressed tar archive of the given entries to path. +func buildTarGz(t *testing.T, path string, entries []rawTarEntry) { + t.Helper() + + file, err := os.Create(path) + require.NoError(t, err) + + defer func() { require.NoError(t, file.Close()) }() + + gzWriter := gzip.NewWriter(file) + tarWriter := tar.NewWriter(gzWriter) + + for _, entry := range entries { + header := &tar.Header{Name: entry.name, Typeflag: entry.typeflag, Mode: 0o644} + + switch entry.typeflag { + case tar.TypeReg: + header.Size = int64(len(entry.content)) + case tar.TypeLink, tar.TypeSymlink: + header.Linkname = entry.linkname + } + + require.NoError(t, tarWriter.WriteHeader(header)) + + if entry.typeflag == tar.TypeReg { + _, writeErr := tarWriter.Write([]byte(entry.content)) + require.NoError(t, writeErr) + } + } + + require.NoError(t, tarWriter.Close()) + require.NoError(t, gzWriter.Close()) +} + +// TestProcessArchive_AppliesMultipleOverlaysInSingleCycle verifies that two +// overlays targeting the same archive are both applied within a single +// extract/repack cycle (the [processArchive] overlay loop). +func TestProcessArchive_AppliesMultipleOverlaysInSingleCycle(t *testing.T) { + ctx := testctx.NewCtx() + sourcesDir := t.TempDir() + + const archiveName = "pkg.tar.gz" + + archivePath := filepath.Join(sourcesDir, archiveName) + + // Single top-level directory so the extract root is "pkg-1.0/". + staging := t.TempDir() + stageFiles(t, staging, map[string]string{ + "pkg-1.0/remove-me.conf": "junk", + "pkg-1.0/keep.conf": "keep", + "pkg-1.0/version.txt": "version = old_value", + }) + require.NoError(t, archive.CreateDeterministicArchive(archivePath, staging, archive.CompressionGzip)) + + // A removal and a search-replace targeting the same archive. Both must be + // applied to the same extracted tree before the single repack. + group := archiveGroup{ + archive: archiveName, + overlays: []projectconfig.ComponentOverlay{ + {Type: projectconfig.ComponentOverlayRemoveFile, Filename: "remove-me.conf"}, + { + Type: projectconfig.ComponentOverlaySearchAndReplaceInFile, + Filename: "version.txt", + Regex: "old_value", + Replacement: "new_value", + }, + }, + } + + repacked, err := processArchive(ctx, sourcesDir, group) + require.NoError(t, err) + assert.True(t, repacked) + + out := t.TempDir() + require.NoError(t, archive.ExtractAuto(archivePath, out)) + + // Removal applied; untouched sibling preserved. + assert.NoFileExists(t, filepath.Join(out, "pkg-1.0", "remove-me.conf")) + assert.FileExists(t, filepath.Join(out, "pkg-1.0", "keep.conf")) + + // Search-replace applied in the same cycle. + content, err := os.ReadFile(filepath.Join(out, "pkg-1.0", "version.txt")) + require.NoError(t, err) + assert.Equal(t, "version = new_value", string(content)) +} + +// TestProcessArchive_ResolveExtractRootFallback drives the extract-root +// resolution through the full [processArchive] cycle, confirming that overlay +// globs are matched relative to the resolved root in both the single-top-level- +// directory case and the multiple-top-level-entries fallback. +func TestProcessArchive_ResolveExtractRootFallback(t *testing.T) { + t.Run("single top-level directory: glob is relative to that directory", func(t *testing.T) { + ctx := testctx.NewCtx() + sourcesDir := t.TempDir() + + const archiveName = "pkg.tar.gz" + + archivePath := filepath.Join(sourcesDir, archiveName) + + staging := t.TempDir() + stageFiles(t, staging, map[string]string{ + "pkg-1.0/remove.conf": "x", + "pkg-1.0/keep.txt": "x", + }) + require.NoError(t, archive.CreateDeterministicArchive(archivePath, staging, archive.CompressionGzip)) + + group := archiveGroup{ + archive: archiveName, + // Path is relative to the extract root (pkg-1.0/), not the archive root. + overlays: []projectconfig.ComponentOverlay{ + {Type: projectconfig.ComponentOverlayRemoveFile, Filename: "remove.conf"}, + }, + } + + repacked, err := processArchive(ctx, sourcesDir, group) + require.NoError(t, err) + assert.True(t, repacked) + assert.Equal(t, []string{"pkg-1.0/keep.txt"}, extractedRegularFiles(t, archivePath)) + }) + + t.Run("multiple top-level entries: glob is relative to the archive root", func(t *testing.T) { + ctx := testctx.NewCtx() + sourcesDir := t.TempDir() + + const archiveName = "pkg.tar.gz" + + archivePath := filepath.Join(sourcesDir, archiveName) + + // Two top-level entries (no single wrapping directory) => extract root is + // the archive root, so the glob is matched there. + staging := t.TempDir() + stageFiles(t, staging, map[string]string{ + "remove.conf": "x", + "keep.txt": "x", + }) + require.NoError(t, archive.CreateDeterministicArchive(archivePath, staging, archive.CompressionGzip)) + + group := archiveGroup{ + archive: archiveName, + overlays: []projectconfig.ComponentOverlay{ + {Type: projectconfig.ComponentOverlayRemoveFile, Filename: "remove.conf"}, + }, + } + + repacked, err := processArchive(ctx, sourcesDir, group) + require.NoError(t, err) + assert.True(t, repacked) + assert.Equal(t, []string{"keep.txt"}, extractedRegularFiles(t, archivePath)) + }) +} + +// TestProcessArchive_UnsupportedEntryTypeErrors verifies the data-loss guard: +// an archive containing an entry that cannot be repacked (here a hardlink) must +// fail rather than silently drop the entry, and the original archive must be +// left untouched (extraction fails before the repack runs). +func TestProcessArchive_UnsupportedEntryTypeErrors(t *testing.T) { + ctx := testctx.NewCtx() + sourcesDir := t.TempDir() + + const archiveName = "pkg.tar.gz" + + archivePath := filepath.Join(sourcesDir, archiveName) + + // CreateDeterministicArchive never emits hardlinks, so build the tarball raw. + buildTarGz(t, archivePath, []rawTarEntry{ + {name: "pkg-1.0/real.txt", typeflag: tar.TypeReg, content: "hello"}, + {name: "pkg-1.0/hard.txt", typeflag: tar.TypeLink, linkname: "pkg-1.0/real.txt"}, + }) + + before, err := os.ReadFile(archivePath) + require.NoError(t, err) + + group := archiveGroup{ + archive: archiveName, + overlays: []projectconfig.ComponentOverlay{ + {Type: projectconfig.ComponentOverlayRemoveFile, Filename: "real.txt"}, + }, + } + + repacked, err := processArchive(ctx, sourcesDir, group) + require.Error(t, err, "an unsupported (hardlink) entry must fail rather than be silently dropped") + assert.False(t, repacked) + assert.Contains(t, err.Error(), "unsupported type") + + // The original archive must be byte-for-byte intact (the repack never ran). + after, err := os.ReadFile(archivePath) + require.NoError(t, err) + assert.Equal(t, before, after, "a failed extraction must not modify the source archive") +} + +// TestProcessArchive_DirectoryHandling pins two documented behaviors of +// file-remove inside an archive: emptied directories survive (file-remove never +// deletes directories), and a bare directory pattern matches nothing and errors. +func TestProcessArchive_DirectoryHandling(t *testing.T) { + t.Run("emptied directory survives file removal", func(t *testing.T) { + ctx := testctx.NewCtx() + sourcesDir := t.TempDir() + + const archiveName = "pkg.tar.gz" + + archivePath := filepath.Join(sourcesDir, archiveName) + + staging := t.TempDir() + stageFiles(t, staging, map[string]string{ + "pkg-1.0/sub/a.txt": "x", + "pkg-1.0/sub/b.txt": "x", + "pkg-1.0/keep.txt": "x", + }) + require.NoError(t, archive.CreateDeterministicArchive(archivePath, staging, archive.CompressionGzip)) + + group := archiveGroup{ + archive: archiveName, + overlays: []projectconfig.ComponentOverlay{ + {Type: projectconfig.ComponentOverlayRemoveFile, Filename: "sub/**"}, + }, + } + + repacked, err := processArchive(ctx, sourcesDir, group) + require.NoError(t, err) + assert.True(t, repacked) + + out := t.TempDir() + require.NoError(t, archive.ExtractAuto(archivePath, out)) + + // Every file under sub/ is gone... + assert.Equal(t, []string{"pkg-1.0/keep.txt"}, listRegularFiles(t, out)) + + // ...but the now-empty directory survives (file-remove can't delete dirs). + info, err := os.Stat(filepath.Join(out, "pkg-1.0", "sub")) + require.NoError(t, err) + assert.True(t, info.IsDir(), "emptied directory should survive file removal") + }) + + t.Run("bare directory pattern matches nothing and errors", func(t *testing.T) { + ctx := testctx.NewCtx() + sourcesDir := t.TempDir() + + const archiveName = "pkg.tar.gz" + + archivePath := filepath.Join(sourcesDir, archiveName) + + staging := t.TempDir() + stageFiles(t, staging, map[string]string{ + "pkg-1.0/sub/a.txt": "x", + }) + require.NoError(t, archive.CreateDeterministicArchive(archivePath, staging, archive.CompressionGzip)) + + group := archiveGroup{ + archive: archiveName, + // A bare directory name: the files-only matcher matches nothing. + overlays: []projectconfig.ComponentOverlay{ + {Type: projectconfig.ComponentOverlayRemoveFile, Filename: "sub"}, + }, + } + + repacked, err := processArchive(ctx, sourcesDir, group) + require.Error(t, err, "removing a directory is unsupported; the pattern should match no files") + assert.False(t, repacked) + }) +} + +// TestProcessArchive_PreservesMislabeledFormatOnRepack verifies that when an +// archive's extension lies about its real format (here a plain, uncompressed tar +// named ".tar.gz"), the repacked archive keeps the real on-disk format (plain +// tar) rather than being "healed" to match the extension. The misleading name is +// preserved; only the contents change. +func TestProcessArchive_PreservesMislabeledFormatOnRepack(t *testing.T) { + ctx := testctx.NewCtx() + sourcesDir := t.TempDir() + + // Misleading name: ".tar.gz" extension, but written as an uncompressed tar. + const archiveName = "pkg.tar.gz" + + archivePath := filepath.Join(sourcesDir, archiveName) + + staging := t.TempDir() + stageFiles(t, staging, map[string]string{ + "pkg-1.0/remove-me.txt": "junk", + "pkg-1.0/keep.txt": "keep", + }) + require.NoError(t, archive.CreateDeterministicArchive(archivePath, staging, archive.CompressionNone)) + + // Precondition: the file really is an uncompressed tar despite its name. + preComp, err := archive.SniffCompressionFromFile(archivePath, archive.CompressionGzip) + require.NoError(t, err) + require.Equal(t, archive.CompressionNone, preComp) + + group := archiveGroup{ + archive: archiveName, + overlays: []projectconfig.ComponentOverlay{ + {Type: projectconfig.ComponentOverlayRemoveFile, Filename: "remove-me.txt"}, + }, + } + + repacked, err := processArchive(ctx, sourcesDir, group) + require.NoError(t, err) + assert.True(t, repacked) + + // The overlay was applied (file removed)... + assert.Equal(t, []string{"pkg-1.0/keep.txt"}, extractedRegularFiles(t, archivePath)) + + // ...and the repacked archive preserves the real format: still an uncompressed + // tar, NOT re-compressed to gzip to match the ".tar.gz" extension. + postComp, err := archive.SniffCompressionFromFile(archivePath, archive.CompressionGzip) + require.NoError(t, err) + assert.Equal(t, archive.CompressionNone, postComp, + "repack must preserve the original (sniffed) format, keeping the misleading extension") +} + +// TestProcessArchive_RepackedArchiveHasNormalPermissions is a regression test: +// the repack writes to an os.CreateTemp placeholder (mode 0600) before renaming +// it over the original. Because CreateDeterministicArchive only truncates an +// existing file, reusing that placeholder would leave the repacked archive at +// 0600. processArchive removes the placeholder first so the archive is recreated +// with normal permissions; assert it does not regress to 0600. +func TestProcessArchive_RepackedArchiveHasNormalPermissions(t *testing.T) { + ctx := testctx.NewCtx() + sourcesDir := t.TempDir() + + const archiveName = "pkg.tar.gz" + + archivePath := filepath.Join(sourcesDir, archiveName) + + staging := t.TempDir() + stageFiles(t, staging, map[string]string{ + "pkg-1.0/remove-me.txt": "junk", + "pkg-1.0/keep.txt": "keep", + }) + require.NoError(t, archive.CreateDeterministicArchive(archivePath, staging, archive.CompressionGzip)) + // Seed the original archive at the restrictive 0600 so the test would fail if + // the repack inherited the original's (or the temp placeholder's) mode instead + // of recreating the file with normal permissions. + require.NoError(t, os.Chmod(archivePath, 0o600)) + + group := archiveGroup{ + archive: archiveName, + overlays: []projectconfig.ComponentOverlay{ + {Type: projectconfig.ComponentOverlayRemoveFile, Filename: "remove-me.txt"}, + }, + } + + repacked, err := processArchive(ctx, sourcesDir, group) + require.NoError(t, err) + require.True(t, repacked) + + // Compare against a freshly os.Create'd file in the same dir: that is exactly + // the mode a normal (non-placeholder) create yields under this umask. This is + // umask-independent and proves the archive was recreated rather than inheriting + // the 0600 placeholder. + refPath := filepath.Join(sourcesDir, "reference") + refFile, err := os.Create(refPath) + require.NoError(t, err) + require.NoError(t, refFile.Close()) + + refInfo, err := os.Stat(refPath) + require.NoError(t, err) + + info, err := os.Stat(archivePath) + require.NoError(t, err) + assert.Equal(t, refInfo.Mode().Perm(), info.Mode().Perm(), + "repacked archive must have normal create permissions, not the 0600 temp-file mode") + assert.NotEqual(t, os.FileMode(0o600), info.Mode().Perm(), + "repacked archive must not inherit the 0600 temp-file permissions") +} diff --git a/internal/app/azldev/core/sources/archiveoverlays_test.go b/internal/app/azldev/core/sources/archiveoverlays_test.go new file mode 100644 index 00000000..85fc1dc8 --- /dev/null +++ b/internal/app/azldev/core/sources/archiveoverlays_test.go @@ -0,0 +1,443 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package sources_test + +import ( + "os" + "path/filepath" + "sort" + "testing" + + "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/core/components/components_testutils" + "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/core/sources" + "github.com/microsoft/azure-linux-dev-tools/internal/global/testctx" + "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" + "github.com/microsoft/azure-linux-dev-tools/internal/providers/sourceproviders" + "github.com/microsoft/azure-linux-dev-tools/internal/providers/sourceproviders/fedorasource" + "github.com/microsoft/azure-linux-dev-tools/internal/providers/sourceproviders/sourceproviders_test" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/archive" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileperms" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileutils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" +) + +// globRemovalTree returns the source tree shared by the file-remove glob tests. +// Every entry is a regular file (the glob matcher used by file-remove is +// files-only). +func globRemovalTree() []string { + return []string{ + "keep.txt", + "some_folder/a.txt", + "some_folder/b.txt", + "some_folder/sub/c.txt", + "some_folder/sub/deep/d.txt", + "top1/nested/some_file.txt", + "top1/some_file.txt", + "top2/some_file.txt", + } +} + +// globRemovalCase describes the expected outcome of applying a file-remove +// overlay with a given glob pattern against [globRemovalTree]. +type globRemovalCase struct { + name string + // pattern is the in-archive / in-sources glob (no archive prefix). + pattern string + // wantErr is true when the pattern matches no files and the overlay errors. + wantErr bool + // remaining is the sorted set of regular files left after removal (equal to + // the whole tree when wantErr is true, since nothing is removed). + remaining []string +} + +// globRemovalCases enumerates the documented file-remove glob behaviors. The +// expectations were captured against the live doublestar matcher and encode two +// behaviors worth pinning: (1) the matcher is files-only, so a bare directory +// name matches nothing, and (2) `*` matches a single path segment while `**` +// matches any depth. Directories are never removed (only files), so emptied +// directories survive a removal; these assertions look at regular files only. +func globRemovalCases() []globRemovalCase { + return []globRemovalCase{ + { + name: "bare folder name matches nothing (files-only matcher)", + pattern: "some_folder", + wantErr: true, + remaining: globRemovalTree(), + }, + { + name: "single star removes immediate file children only", + pattern: "some_folder/*", + remaining: []string{ + "keep.txt", + "some_folder/sub/c.txt", + "some_folder/sub/deep/d.txt", + "top1/nested/some_file.txt", + "top1/some_file.txt", + "top2/some_file.txt", + }, + }, + { + name: "doublestar removes all files under the folder recursively", + pattern: "some_folder/**", + remaining: []string{ + "keep.txt", + "top1/nested/some_file.txt", + "top1/some_file.txt", + "top2/some_file.txt", + }, + }, + { + name: "single-star prefix matches only top-level folders", + pattern: "*/some_file.txt", + remaining: []string{ + "keep.txt", + "some_folder/a.txt", + "some_folder/b.txt", + "some_folder/sub/c.txt", + "some_folder/sub/deep/d.txt", + "top1/nested/some_file.txt", + }, + }, + { + name: "doublestar prefix matches files at any depth", + pattern: "**/some_file.txt", + remaining: []string{ + "keep.txt", + "some_folder/a.txt", + "some_folder/b.txt", + "some_folder/sub/c.txt", + "some_folder/sub/deep/d.txt", + }, + }, + } +} + +// writeTreeFiles materializes the given slash-separated relative file paths +// under root, each with placeholder content. +func writeTreeFiles(t *testing.T, root string, files []string) { + t.Helper() + + for _, f := range files { + full := filepath.Join(root, filepath.FromSlash(f)) + require.NoError(t, os.MkdirAll(filepath.Dir(full), fileperms.PublicDir)) + require.NoError(t, os.WriteFile(full, []byte("x"), fileperms.PrivateFile)) + } +} + +// collectRegularFiles returns the sorted, slash-separated relative paths of all +// regular files under root (directories are ignored). +func collectRegularFiles(t *testing.T, root string) []string { + t.Helper() + + var out []string + + require.NoError(t, filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error { + if err != nil { + return err + } + + if d.IsDir() { + return nil + } + + rel, relErr := filepath.Rel(root, path) + if relErr != nil { + return relErr + } + + out = append(out, filepath.ToSlash(rel)) + + return nil + })) + + sort.Strings(out) + + return out +} + +// TestPrepareSources_RemoveFileGlob_Archive exercises archive-scoped file-remove +// globs through the real exported [sources.SourcePreparer.PrepareSources] path +// (which drives the extract/remove/repack cycle), asserting which files survive +// inside the repacked archive for each glob pattern. +func TestPrepareSources_RemoveFileGlob_Archive(t *testing.T) { + const ( + componentName = "test-component" + archiveName = "pkg.tar.gz" + ) + + for _, testCase := range globRemovalCases() { + t.Run(testCase.name, func(t *testing.T) { + // Host FS + real temp dir: archive extraction/repacking happens on disk. + ctx := testctx.NewCtx(testctx.WithHostFS()) + outputDir := t.TempDir() + archivePath := filepath.Join(outputDir, archiveName) + + // Stage a tree with multiple top-level entries so the extraction root + // is the archive root and the glob is matched relative to it. + staging := t.TempDir() + writeTreeFiles(t, staging, globRemovalTree()) + require.NoError(t, archive.CreateDeterministicArchive(archivePath, staging, archive.CompressionGzip)) + + // Seed a 'sources' entry for the archive so the post-overlay rehash + // has an entry to update (a missing entry is itself an error). + originalHash, err := fileutils.ComputeFileHash(ctx.FS(), fileutils.HashTypeSHA256, archivePath) + require.NoError(t, err) + + sourcesPath := filepath.Join(outputDir, fedorasource.SourcesFileName) + entry := fedorasource.FormatSourcesEntry(archiveName, fileutils.HashTypeSHA256, originalHash) + require.NoError(t, fileutils.WriteFile( + ctx.FS(), sourcesPath, []byte(entry+"\n"), fileperms.PublicFile)) + + ctrl := gomock.NewController(t) + component := components_testutils.NewMockComponent(ctrl) + sourceManager := sourceproviders_test.NewMockSourceManager(ctrl) + + component.EXPECT().GetName().AnyTimes().Return(componentName) + component.EXPECT().GetConfig().AnyTimes().Return(&projectconfig.ComponentConfig{ + Overlays: []projectconfig.ComponentOverlay{ + { + Type: projectconfig.ComponentOverlayRemoveFile, + Archive: archiveName, + Filename: testCase.pattern, + }, + }, + }) + + sourceManager.EXPECT().FetchFiles(gomock.Any(), component, outputDir).Return(nil) + sourceManager.EXPECT().FetchComponent(gomock.Any(), component, outputDir, gomock.Any()).DoAndReturn( + func(_ interface{}, _ interface{}, dir string, _ ...sourceproviders.FetchComponentOption) error { + return fileutils.WriteFile( + ctx.FS(), filepath.Join(dir, componentName+".spec"), + []byte("# test spec"), fileperms.PublicFile) + }, + ) + + preparer, err := sources.NewPreparer(sourceManager, ctx.FS(), ctx, ctx) + require.NoError(t, err) + + err = preparer.PrepareSources(ctx, component, outputDir, true /*applyOverlays*/) + + if testCase.wantErr { + // A no-match pattern errors and leaves the archive untouched. + require.Error(t, err) + + return + } + + require.NoError(t, err) + + out := t.TempDir() + require.NoError(t, archive.ExtractAuto(archivePath, out)) + assert.Equal(t, testCase.remaining, collectRegularFiles(t, out)) + }) + } +} + +// TestApplyOverlayToSources_RemoveFileGlob_LooseFiles exercises the same glob +// patterns against loose files in the sources tree (no archive prefix) through +// the real exported [sources.ApplyOverlayToSources] entry point, confirming +// archive-scoped and loose-file removal share identical glob semantics. +func TestApplyOverlayToSources_RemoveFileGlob_LooseFiles(t *testing.T) { + for _, testCase := range globRemovalCases() { + t.Run(testCase.name, func(t *testing.T) { + ctx := testctx.NewCtx(testctx.WithHostFS()) + sourcesDir := t.TempDir() + writeTreeFiles(t, sourcesDir, globRemovalTree()) + + overlay := projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayRemoveFile, + Filename: testCase.pattern, + } + + // file-remove does not touch the spec, so specPath is unused. + err := sources.ApplyOverlayToSources(ctx, ctx.FS(), overlay, sourcesDir, "") + + if testCase.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + + assert.Equal(t, testCase.remaining, collectRegularFiles(t, sourcesDir)) + }) + } +} + +// findSourcesEntry returns the parsed 'sources' entry for filename, or nil. +func findSourcesEntry(t *testing.T, sourcesContent, filename string) *fedorasource.SourcesFileEntry { + t.Helper() + + parsed, err := fedorasource.ReadSourcesFile(sourcesContent) + require.NoError(t, err) + + for i := range parsed { + if parsed[i].Entry != nil && parsed[i].Entry.Filename == filename { + return parsed[i].Entry + } + } + + return nil +} + +// TestPrepareSources_SearchReplaceInArchiveRehashesEntry is an end-to-end check +// that a file-search-replace overlay scoped to an archive rewrites the file +// inside the archive, repacks it, and rehashes the matching 'sources' entry +// (preserving the original hash type). file-remove already has this coverage; +// this exercises the search-replace path through the exported PrepareSources. +func TestPrepareSources_SearchReplaceInArchiveRehashesEntry(t *testing.T) { + const ( + componentName = "test-component" + archiveName = "pkg.tar.gz" + ) + + // Host FS + real temp dir: archive extraction/repacking happens on disk. + ctx := testctx.NewCtx(testctx.WithHostFS()) + outputDir := t.TempDir() + archivePath := filepath.Join(outputDir, archiveName) + + // Single top-level directory => extract root is "pkg-1.0/", so the inner + // path "configure.ac" resolves relative to that root. + staging := t.TempDir() + pkgRoot := filepath.Join(staging, "pkg-1.0") + require.NoError(t, os.MkdirAll(pkgRoot, fileperms.PublicDir)) + require.NoError(t, os.WriteFile(filepath.Join(pkgRoot, "configure.ac"), + []byte("AC_CHECK_LIB(old_lib, main)\n"), fileperms.PrivateFile)) + require.NoError(t, archive.CreateDeterministicArchive(archivePath, staging, archive.CompressionGzip)) + + // Seed a SHA256 'sources' entry (not the SHA512 default) so the test also + // proves the hash type is preserved. + originalHash, err := fileutils.ComputeFileHash(ctx.FS(), fileutils.HashTypeSHA256, archivePath) + require.NoError(t, err) + + sourcesPath := filepath.Join(outputDir, fedorasource.SourcesFileName) + entry := fedorasource.FormatSourcesEntry(archiveName, fileutils.HashTypeSHA256, originalHash) + require.NoError(t, fileutils.WriteFile(ctx.FS(), sourcesPath, []byte(entry+"\n"), fileperms.PublicFile)) + + ctrl := gomock.NewController(t) + component := components_testutils.NewMockComponent(ctrl) + sourceManager := sourceproviders_test.NewMockSourceManager(ctrl) + + component.EXPECT().GetName().AnyTimes().Return(componentName) + component.EXPECT().GetConfig().AnyTimes().Return(&projectconfig.ComponentConfig{ + Overlays: []projectconfig.ComponentOverlay{ + { + Type: projectconfig.ComponentOverlaySearchAndReplaceInFile, + Archive: archiveName, + Filename: "configure.ac", + Regex: "old_lib", + Replacement: "new_lib", + }, + }, + }) + + sourceManager.EXPECT().FetchFiles(gomock.Any(), component, outputDir).Return(nil) + sourceManager.EXPECT().FetchComponent(gomock.Any(), component, outputDir, gomock.Any()).DoAndReturn( + func(_ interface{}, _ interface{}, dir string, _ ...sourceproviders.FetchComponentOption) error { + return fileutils.WriteFile(ctx.FS(), filepath.Join(dir, componentName+".spec"), + []byte("# test spec"), fileperms.PublicFile) + }, + ) + + preparer, err := sources.NewPreparer(sourceManager, ctx.FS(), ctx, ctx) + require.NoError(t, err) + + require.NoError(t, preparer.PrepareSources(ctx, component, outputDir, true /*applyOverlays*/)) + + // Rewrite: the file content inside the repacked archive reflects the replacement. + out := t.TempDir() + require.NoError(t, archive.ExtractAuto(archivePath, out)) + content, err := os.ReadFile(filepath.Join(out, "pkg-1.0", "configure.ac")) + require.NoError(t, err) + assert.Equal(t, "AC_CHECK_LIB(new_lib, main)\n", string(content)) + + // Repack: the archive's hash changed. + repackedHash, err := fileutils.ComputeFileHash(ctx.FS(), fileutils.HashTypeSHA256, archivePath) + require.NoError(t, err) + require.NotEqual(t, originalHash, repackedHash, + "precondition: rewriting a file in the archive should change its hash") + + // Rehash: the 'sources' entry was rewritten to the repacked hash, type preserved. + sourcesContent, err := fileutils.ReadFile(ctx.FS(), sourcesPath) + require.NoError(t, err) + + got := findSourcesEntry(t, string(sourcesContent), archiveName) + require.NotNil(t, got, "rewritten 'sources' file should still contain an entry for %q", archiveName) + assert.Equal(t, fileutils.HashTypeSHA256, got.HashType, "original hash type must be preserved") + assert.Equal(t, repackedHash, got.Hash, "'sources' entry must record the repacked archive's hash") +} + +// TestPrepareSources_SkipSourcesSkipsArchiveOverlays verifies the --skip-sources +// branch: when source downloads are skipped, archive overlays are skipped (with a +// warning) instead of applied, leaving both the archive and its 'sources' entry +// untouched. +func TestPrepareSources_SkipSourcesSkipsArchiveOverlays(t *testing.T) { + const ( + componentName = "test-component" + archiveName = "pkg.tar.gz" + ) + + ctx := testctx.NewCtx(testctx.WithHostFS()) + outputDir := t.TempDir() + archivePath := filepath.Join(outputDir, archiveName) + + staging := t.TempDir() + pkgRoot := filepath.Join(staging, "pkg-1.0") + require.NoError(t, os.MkdirAll(pkgRoot, fileperms.PublicDir)) + require.NoError(t, os.WriteFile(filepath.Join(pkgRoot, "remove-me.txt"), + []byte("delete me"), fileperms.PrivateFile)) + require.NoError(t, archive.CreateDeterministicArchive(archivePath, staging, archive.CompressionGzip)) + + // Snapshot the archive and seed its 'sources' entry so we can assert both + // are left untouched. + originalArchive, err := os.ReadFile(archivePath) + require.NoError(t, err) + + originalHash, err := fileutils.ComputeFileHash(ctx.FS(), fileutils.HashTypeSHA256, archivePath) + require.NoError(t, err) + + sourcesPath := filepath.Join(outputDir, fedorasource.SourcesFileName) + entry := fedorasource.FormatSourcesEntry(archiveName, fileutils.HashTypeSHA256, originalHash) + require.NoError(t, fileutils.WriteFile(ctx.FS(), sourcesPath, []byte(entry+"\n"), fileperms.PublicFile)) + + ctrl := gomock.NewController(t) + component := components_testutils.NewMockComponent(ctrl) + sourceManager := sourceproviders_test.NewMockSourceManager(ctrl) + + component.EXPECT().GetName().AnyTimes().Return(componentName) + component.EXPECT().GetConfig().AnyTimes().Return(&projectconfig.ComponentConfig{ + Overlays: []projectconfig.ComponentOverlay{ + {Type: projectconfig.ComponentOverlayRemoveFile, Archive: archiveName, Filename: "remove-me.txt"}, + }, + }) + + // With --skip-sources, FetchFiles must NOT be called (no EXPECT for it); + // FetchComponent is still called to provide the spec. + sourceManager.EXPECT().FetchComponent(gomock.Any(), component, outputDir, gomock.Any()).DoAndReturn( + func(_ interface{}, _ interface{}, dir string, _ ...sourceproviders.FetchComponentOption) error { + return fileutils.WriteFile(ctx.FS(), filepath.Join(dir, componentName+".spec"), + []byte("# test spec"), fileperms.PublicFile) + }, + ) + + preparer, err := sources.NewPreparer(sourceManager, ctx.FS(), ctx, ctx, sources.WithSkipLookaside()) + require.NoError(t, err) + + require.NoError(t, preparer.PrepareSources(ctx, component, outputDir, true /*applyOverlays*/)) + + // The archive overlay is skipped: the archive is byte-for-byte unchanged... + afterArchive, err := os.ReadFile(archivePath) + require.NoError(t, err) + assert.Equal(t, originalArchive, afterArchive, + "archive must not be modified when --skip-sources skips archive overlays") + + // ...and its 'sources' entry keeps the original hash. + sourcesContent, err := fileutils.ReadFile(ctx.FS(), sourcesPath) + require.NoError(t, err) + + got := findSourcesEntry(t, string(sourcesContent), archiveName) + require.NotNil(t, got) + assert.Equal(t, originalHash, got.Hash, "'sources' entry hash must be unchanged when overlays are skipped") +} diff --git a/internal/app/azldev/core/sources/overlays.go b/internal/app/azldev/core/sources/overlays.go index 66cd2373..44c2ca5c 100644 --- a/internal/app/azldev/core/sources/overlays.go +++ b/internal/app/azldev/core/sources/overlays.go @@ -53,8 +53,8 @@ func ApplyOverlayToSources( } } - // Apply the non-spec file component, if any. - if !overlay.ModifiesNonSpecFiles() { + // Apply the loose-file component, if any. + if !overlay.ModifiesLooseFiles() { return nil } diff --git a/internal/app/azldev/core/sources/sourceprep.go b/internal/app/azldev/core/sources/sourceprep.go index 617dcbd8..fb16fd4c 100644 --- a/internal/app/azldev/core/sources/sourceprep.go +++ b/internal/app/azldev/core/sources/sourceprep.go @@ -246,12 +246,12 @@ func (p *sourcePreparerImpl) PrepareSources( } if applyOverlays { - err := p.applyOverlaysToSources(ctx, component, outputDir) + repackedArchives, err := p.applyOverlaysToSources(component, outputDir) if err != nil { return err } - if err := p.updateSourcesFile(component, outputDir); err != nil { + if err := p.updateSourcesFile(component, outputDir, repackedArchives); err != nil { return fmt.Errorf("failed to update 'sources' file for component %#q:\n%w", component.GetName(), err) } @@ -273,17 +273,17 @@ func (p *sourcePreparerImpl) PrepareSources( } // applyOverlaysToSources writes the macros file and then applies all overlays. +// It returns the names of any archives that were repacked by archive overlays +// (empty in dry-run mode or when no archive overlays ran), so the caller can +// rehash exactly those entries in the 'sources' file. func (p *sourcePreparerImpl) applyOverlaysToSources( - ctx context.Context, component components.Component, outputDir string, -) error { - // Emit computed macros to a macros file in the output directory. - // If the build configuration produces no macros, no file is written and - // macrosFileName will be empty. + component components.Component, outputDir string, +) ([]string, error) { var macrosFileName string macrosFilePath, err := p.writeMacrosFile(component, outputDir) if err != nil { - return fmt.Errorf("failed to write macros file for component %#q:\n%w", + return nil, fmt.Errorf("failed to write macros file for component %#q:\n%w", component.GetName(), err) } @@ -291,47 +291,88 @@ func (p *sourcePreparerImpl) applyOverlaysToSources( macrosFileName = filepath.Base(macrosFilePath) } - // Apply all overlays to prepared sources. - if err := p.applyOverlays(ctx, component, outputDir, macrosFileName); err != nil { - return fmt.Errorf("failed to apply overlays for component %#q:\n%w", component.GetName(), err) + repackedArchives, err := p.applyOverlays(component, outputDir, macrosFileName) + if err != nil { + return nil, fmt.Errorf("failed to apply overlays for component %#q:\n%w", + component.GetName(), err) } - return nil + return repackedArchives, nil } // applyOverlays applies all overlays (user-defined and system-generated) to the -// component sources. Overlay application is decoupled from git history generation: -// overlays modify the working tree; synthetic history is recorded separately by -// [trySyntheticHistory]. +// component sources. It returns the names of any archives that were repacked by +// archive overlays. func (p *sourcePreparerImpl) applyOverlays( - _ context.Context, component components.Component, sourcesDirPath, macrosFileName string, -) error { + component components.Component, sourcesDirPath, macrosFileName string, +) ([]string, error) { event := p.eventListener.StartEvent("Applying overlays", "component", component.GetName()) defer event.End() - // Resolve the spec path once for all overlay operations in this call. absSpecPath, err := p.resolveSpecPath(component, sourcesDirPath) if err != nil { - return err + return nil, err } - // Collect all overlays in application order. This ensures every change is - // captured in the synthetic history, including build configuration changes. allOverlays, err := p.collectOverlays(component, macrosFileName) if err != nil { - return fmt.Errorf("failed to collect overlays for component %#q:\n%w", component.GetName(), err) + return nil, fmt.Errorf("failed to collect overlays for component %#q:\n%w", component.GetName(), err) } if len(allOverlays) == 0 { - return nil + return nil, nil + } + + // Archive overlays are applied first (they modify archived source files + // in-place), followed by spec and loose-file overlays. Each function + // self-filters to the overlay types it handles. + repackedArchives, err := p.applyArchiveOverlayGroup(component, sourcesDirPath, allOverlays) + if err != nil { + return nil, err } - // Apply all overlays to the working tree. if err := p.applyOverlayList(allOverlays, sourcesDirPath, absSpecPath); err != nil { - return fmt.Errorf("failed to apply overlays for component %#q:\n%w", component.GetName(), err) + return nil, fmt.Errorf("failed to apply overlays for component %#q:\n%w", component.GetName(), err) } - return nil + return repackedArchives, nil +} + +// applyArchiveOverlayGroup applies the archive-scoped overlays contained in the +// given overlay list. The list may hold overlays of any type; only those for +// which [projectconfig.ComponentOverlay.ModifiesArchive] reports true are +// processed here. Skipped when source downloads were not performed. It returns +// the names of the archives that were actually repacked (empty in dry-run mode +// or when source downloads were skipped). +func (p *sourcePreparerImpl) applyArchiveOverlayGroup( + component components.Component, + sourcesDirPath string, overlays []projectconfig.ComponentOverlay, +) ([]string, error) { + archiveOverlays := lo.Filter(overlays, func(overlay projectconfig.ComponentOverlay, _ int) bool { + return overlay.ModifiesArchive() + }) + + if len(archiveOverlays) == 0 { + return nil, nil + } + + if p.skipLookaside { + slog.Warn("Skipping archive overlays because source downloads were skipped (--skip-sources)", + "component", component.GetName(), + "count", len(archiveOverlays)) + + return nil, nil + } + + repackedArchives, err := applyArchiveOverlays( + p.dryRunnable, p.eventListener, sourcesDirPath, archiveOverlays, + ) + if err != nil { + return nil, fmt.Errorf("failed to apply archive overlays for component %#q:\n%w", + component.GetName(), err) + } + + return repackedArchives, nil } // collectOverlays gathers all overlays for a component into a single ordered slice: @@ -604,8 +645,10 @@ func (p *sourcePreparerImpl) DiffSources( return nil, fmt.Errorf("failed to copy sources for component %#q:\n%w", component.GetName(), err) } - // Apply overlays in-place to the copied directory only. - if err := p.applyOverlaysToSources(ctx, component, overlaidDir); err != nil { + // Apply overlays in-place to the copied directory only. The repacked-archive + // list is unused here: DiffSources diffs the trees directly and does not + // rewrite a 'sources' file. + if _, err := p.applyOverlaysToSources(component, overlaidDir); err != nil { return nil, fmt.Errorf("failed to apply overlays for component %#q:\n%w", component.GetName(), err) } @@ -634,9 +677,17 @@ func (p *sourcePreparerImpl) DiffSources( // enforced by [projectconfig.ConfigFile.Validate]). Setting `ReplaceUpstream` = true without // a matching upstream entry is also an error: the user expressed intent to replace something // that isn't there, which almost certainly indicates a stale config or filename typo. -func (p *sourcePreparerImpl) updateSourcesFile(component components.Component, outputDir string) error { - sourceFiles := component.GetConfig().SourceFiles - if len(sourceFiles) == 0 { +func (p *sourcePreparerImpl) updateSourcesFile( + component components.Component, outputDir string, modifiedArchives []string, +) error { + config := component.GetConfig() + sourceFiles := config.SourceFiles + + // modifiedArchives lists the archives that archive overlays actually repacked + // during this run; their 'sources' digests must be refreshed. The list is empty + // when no archive overlays ran, in dry-run mode, or when source downloads were + // skipped, so rehashing is correctly avoided in those cases. + if len(sourceFiles) == 0 && len(modifiedArchives) == 0 { return nil } @@ -647,7 +698,19 @@ func (p *sourcePreparerImpl) updateSourcesFile(component components.Component, o return err } - mergedLines, err := p.buildSourceEntries(sourceFiles, existingContent, component.GetName(), outputDir) + // Parse once, then rehash modified archives and merge source-files entries + // on the parsed representation — single parse, single write. + existingLines, err := fedorasource.ReadSourcesFile(existingContent) + if err != nil { + return fmt.Errorf("failed to parse 'sources' file %#q:\n%w", sourcesFilePath, err) + } + + // Rehash archives that were modified by archive overlays in-place. + if err := p.rehashModifiedEntries(existingLines, outputDir, modifiedArchives); err != nil { + return err + } + + mergedLines, err := p.buildSourceEntries(sourceFiles, existingLines, component.GetName(), outputDir) if err != nil { return err } @@ -666,6 +729,75 @@ func (p *sourcePreparerImpl) updateSourcesFile(component components.Component, o return nil } +// rehashModifiedEntries updates the Raw and Entry fields of parsed 'sources' lines +// for archives that were modified by archive overlays. The hash is recomputed using +// the same hash type as the original entry. It returns an error if any modified +// archive has no matching 'sources' entry, since that would leave a stale digest. +func (p *sourcePreparerImpl) rehashModifiedEntries( + lines []fedorasource.SourcesFileLine, outputDir string, modifiedArchives []string, +) error { + if len(modifiedArchives) == 0 { + return nil + } + + // Track which archives we actually rehashed so we can detect any that were + // repacked by an overlay but have no matching 'sources' entry. Leaving such + // an archive unrehashed would record a stale digest, so it is treated as an error. + rehashed := make(map[string]bool, len(modifiedArchives)) + for _, name := range modifiedArchives { + rehashed[name] = false + } + + for idx, line := range lines { + if line.Entry == nil { + continue + } + + if _, ok := rehashed[line.Entry.Filename]; !ok { + continue + } + + archivePath := filepath.Join(outputDir, line.Entry.Filename) + + newHash, err := fileutils.ComputeFileHash(p.fs, line.Entry.HashType, archivePath) + if err != nil { + return fmt.Errorf("rehashing modified archive %#q:\n%w", line.Entry.Filename, err) + } + + slog.Debug("Rehashed modified archive in 'sources' file", + "archive", line.Entry.Filename, + "hashType", line.Entry.HashType, + "oldHash", line.Entry.Hash, + "newHash", newHash, + ) + + lines[idx].Raw = fedorasource.FormatSourcesEntry(line.Entry.Filename, line.Entry.HashType, newHash) + lines[idx].Entry.Hash = newHash + rehashed[line.Entry.Filename] = true + } + + // Any archive that an overlay repacked but that has no 'sources' entry would + // silently keep a stale digest. Surface this as an error identifying the + // missing filenames rather than producing an inconsistent 'sources' file. + var missing []string + + for name, done := range rehashed { + if !done { + missing = append(missing, name) + } + } + + if len(missing) > 0 { + slices.Sort(missing) + + return fmt.Errorf( + "archive overlay(s) modified %d archive(s) with no matching 'sources' entry to rehash: %s", + len(missing), strings.Join(missing, ", ")) + } + + return nil +} + // readSourcesFileIfExists reads the 'sources' file content if it exists, returning empty string if not. func (p *sourcePreparerImpl) readSourcesFileIfExists(sourcesFilePath string) (string, error) { exists, err := fileutils.Exists(p.fs, sourcesFilePath) @@ -685,32 +817,24 @@ func (p *sourcePreparerImpl) readSourcesFileIfExists(sourcesFilePath string) (st return string(data), nil } -// buildSourceEntries validates [projectconfig.SourceFileReference] entries and returns -// the merged set of lines ready to be written to the 'sources' file. Before returning, -// it logs an INFO-level event indicating that the 'sources' file will be updated, -// including the counts of newly added and replaced entries. +// buildSourceEntries merges user-declared [projectconfig.SourceFileReference] entries +// into the parsed 'sources' lines. Returns the final set of raw lines ready to be +// written to the 'sources' file. // // Output ordering and preservation: -// - Each line of [existingContent] is emitted verbatim, except for entry lines whose +// - Each existing line is emitted verbatim, except for entry lines whose // filename matches a replacement, which are swapped for the new formatted entry. -// Comments and blank lines from the original file are kept in their original positions. -// - Brand-new entries (no upstream filename collision) are appended after the upstream -// content in the order they appear in [sourceFiles]. +// Comments and blank lines are kept in their original positions. +// - Brand-new entries (no upstream filename collision) are appended after the +// existing content in the order they appear in [sourceFiles]. // // Collision rules and hash resolution are documented on [sourcePreparerImpl.processSourceRef]. func (p *sourcePreparerImpl) buildSourceEntries( sourceFiles []projectconfig.SourceFileReference, - existingContent string, + existingLines []fedorasource.SourcesFileLine, componentName string, outputDir string, ) (mergedLines []string, err error) { - existingLines, err := fedorasource.ReadSourcesFile(existingContent) - if err != nil { - return nil, fmt.Errorf( - "failed to parse existing 'sources' file at %#q:\n%w", - filepath.Join(outputDir, fedorasource.SourcesFileName), err) - } - // Index upstream entries by filename for O(1) collision lookup. The parser // (fedorasource.ReadSourcesFile) errors on duplicate filenames, so the // entries are guaranteed unique by the time we get here. @@ -1085,10 +1209,17 @@ func (p *sourcePreparerImpl) resolveSpecPath( } // applyOverlayList applies a list of overlays to the component sources sequentially. +// Archive-scoped overlays (see [projectconfig.ComponentOverlay.ModifiesArchive]) are +// skipped here; they are handled separately by [applyArchiveOverlays], which batches +// extraction and repacking per archive. func (p *sourcePreparerImpl) applyOverlayList( overlays []projectconfig.ComponentOverlay, sourcesDirPath, absSpecPath string, ) error { for _, overlay := range overlays { + if overlay.ModifiesArchive() { + continue + } + if err := ApplyOverlayToSources( p.dryRunnable, p.fs, overlay, sourcesDirPath, absSpecPath, ); err != nil { diff --git a/internal/app/azldev/core/sources/sourceprep_test.go b/internal/app/azldev/core/sources/sourceprep_test.go index 45a9d286..15a175b2 100644 --- a/internal/app/azldev/core/sources/sourceprep_test.go +++ b/internal/app/azldev/core/sources/sourceprep_test.go @@ -5,6 +5,7 @@ package sources_test import ( "errors" + "os" "path/filepath" "strings" "testing" @@ -16,6 +17,7 @@ import ( "github.com/microsoft/azure-linux-dev-tools/internal/providers/sourceproviders" "github.com/microsoft/azure-linux-dev-tools/internal/providers/sourceproviders/fedorasource" "github.com/microsoft/azure-linux-dev-tools/internal/providers/sourceproviders/sourceproviders_test" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/archive" "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileperms" "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileutils" "github.com/stretchr/testify/assert" @@ -99,6 +101,178 @@ func TestPrepareSources_Success(t *testing.T) { assert.NotContains(t, string(specContents), "Source9999") } +// TestPrepareSources_ArchiveOverlayRehashesSourcesEntry is an end-to-end check +// of the key correctness behavior introduced with archive overlays: when an +// archive-scoped overlay mutates an archive's contents, the matching 'sources' +// entry must be re-hashed in place so the recorded digest reflects the repacked +// archive (while keeping the original hash *type*). +// +// This runs against the host filesystem with a real temp dir because archive +// overlays extract/repack through the [archive] package, which uses OS +// primitives ([os.Root], os.*) and therefore requires genuine on-disk paths — +// an in-memory FS would not be visible to extraction/repacking. This mirrors +// the existing archive internal tests, which likewise use t.TempDir(). +func TestPrepareSources_ArchiveOverlayRehashesSourcesEntry(t *testing.T) { + const ( + componentName = "test-component" + archiveName = "pkg-1.0.tar.gz" + ) + + // Host FS + real temp dir: archive extraction/repacking happens on disk. + ctx := testctx.NewCtx(testctx.WithHostFS()) + outputDir := t.TempDir() + + archivePath := filepath.Join(outputDir, archiveName) + specPath := filepath.Join(outputDir, componentName+".spec") + sourcesPath := filepath.Join(outputDir, fedorasource.SourcesFileName) + + // Build a deterministic archive whose single top-level directory follows the + // conventional "%{name}-%{version}/" layout, containing a file we will remove + // and one we will keep. + stagingDir := t.TempDir() + pkgRoot := filepath.Join(stagingDir, "pkg-1.0") + require.NoError(t, os.MkdirAll(pkgRoot, fileperms.PublicDir)) + require.NoError(t, os.WriteFile(filepath.Join(pkgRoot, "keep.txt"), []byte("keep me"), fileperms.PrivateFile)) + require.NoError(t, os.WriteFile(filepath.Join(pkgRoot, "remove-me.txt"), []byte("delete me"), fileperms.PrivateFile)) + require.NoError(t, archive.CreateDeterministicArchive(archivePath, stagingDir, archive.CompressionGzip)) + + // Record the original hash of the archive and seed a 'sources' file with it. + // Use SHA256 (not the SHA512 default) so the test also proves the hash *type* + // is preserved rather than coincidentally matching a default. + originalHash, err := fileutils.ComputeFileHash(ctx.FS(), fileutils.HashTypeSHA256, archivePath) + require.NoError(t, err) + + originalEntry := fedorasource.FormatSourcesEntry(archiveName, fileutils.HashTypeSHA256, originalHash) + require.NoError(t, fileutils.WriteFile( + ctx.FS(), sourcesPath, []byte(originalEntry+"\n"), fileperms.PublicFile)) + + ctrl := gomock.NewController(t) + component := components_testutils.NewMockComponent(ctrl) + sourceManager := sourceproviders_test.NewMockSourceManager(ctrl) + + component.EXPECT().GetName().AnyTimes().Return(componentName) + component.EXPECT().GetConfig().AnyTimes().Return(&projectconfig.ComponentConfig{ + Overlays: []projectconfig.ComponentOverlay{ + { + Type: projectconfig.ComponentOverlayRemoveFile, + Archive: archiveName, + Filename: "remove-me.txt", + }, + }, + }) + + // The archive and 'sources' file already exist on disk; the source manager + // only needs to provide the spec file (FetchFiles is a no-op download). + sourceManager.EXPECT().FetchFiles(gomock.Any(), component, outputDir).Return(nil) + sourceManager.EXPECT().FetchComponent(gomock.Any(), component, outputDir, gomock.Any()).DoAndReturn( + func(_ interface{}, _ interface{}, dir string, _ ...sourceproviders.FetchComponentOption) error { + return fileutils.WriteFile( + ctx.FS(), filepath.Join(dir, componentName+".spec"), + []byte("# test spec"), fileperms.PublicFile) + }, + ) + + preparer, err := sources.NewPreparer(sourceManager, ctx.FS(), ctx, ctx) + require.NoError(t, err) + + err = preparer.PrepareSources(ctx, component, outputDir, true /*applyOverlays*/) + require.NoError(t, err) + + // The overlay must have actually mutated the archive on disk. + assert.FileExists(t, specPath) + + repackedHash, err := fileutils.ComputeFileHash(ctx.FS(), fileutils.HashTypeSHA256, archivePath) + require.NoError(t, err) + require.NotEqual(t, originalHash, repackedHash, + "precondition: removing a file from the archive should change its hash") + + // The 'sources' entry must have been rewritten to the repacked archive's hash, + // preserving the original SHA256 hash type. + sourcesContent, err := fileutils.ReadFile(ctx.FS(), sourcesPath) + require.NoError(t, err) + + parsedLines, err := fedorasource.ReadSourcesFile(string(sourcesContent)) + require.NoError(t, err) + + var entry *fedorasource.SourcesFileEntry + + for i := range parsedLines { + if parsedLines[i].Entry != nil && parsedLines[i].Entry.Filename == archiveName { + entry = parsedLines[i].Entry + + break + } + } + + require.NotNil(t, entry, "rewritten 'sources' file should still contain an entry for %q", archiveName) + assert.Equal(t, fileutils.HashTypeSHA256, entry.HashType, "original hash type must be preserved") + assert.Equal(t, repackedHash, entry.Hash, "'sources' entry must record the repacked archive's hash") + assert.NotEqual(t, originalHash, entry.Hash, "'sources' entry hash must have been updated") +} + +// TestPrepareSources_ArchiveOverlayMissingSourcesEntryErrors verifies that when an +// archive-scoped overlay repacks an archive that has no matching 'sources' entry, +// preparation fails instead of silently leaving a stale (or absent) digest. +func TestPrepareSources_ArchiveOverlayMissingSourcesEntryErrors(t *testing.T) { + const ( + componentName = "test-component" + archiveName = "pkg-1.0.tar.gz" + ) + + // Host FS + real temp dir: archive extraction/repacking happens on disk. + ctx := testctx.NewCtx(testctx.WithHostFS()) + outputDir := t.TempDir() + + archivePath := filepath.Join(outputDir, archiveName) + sourcesPath := filepath.Join(outputDir, fedorasource.SourcesFileName) + + // Build a deterministic archive with a file to remove, but deliberately seed a + // 'sources' file that has NO entry for this archive. + stagingDir := t.TempDir() + pkgRoot := filepath.Join(stagingDir, "pkg-1.0") + require.NoError(t, os.MkdirAll(pkgRoot, fileperms.PublicDir)) + require.NoError(t, os.WriteFile(filepath.Join(pkgRoot, "remove-me.txt"), []byte("delete me"), fileperms.PrivateFile)) + require.NoError(t, archive.CreateDeterministicArchive(archivePath, stagingDir, archive.CompressionGzip)) + + // 'sources' file references some unrelated file, not the archive being modified. + require.NoError(t, fileutils.WriteFile( + ctx.FS(), sourcesPath, + []byte("SHA256 (unrelated.tar.gz) = 0000000000000000000000000000000000000000000000000000000000000000\n"), + fileperms.PublicFile)) + + ctrl := gomock.NewController(t) + component := components_testutils.NewMockComponent(ctrl) + sourceManager := sourceproviders_test.NewMockSourceManager(ctrl) + + component.EXPECT().GetName().AnyTimes().Return(componentName) + component.EXPECT().GetConfig().AnyTimes().Return(&projectconfig.ComponentConfig{ + Overlays: []projectconfig.ComponentOverlay{ + { + Type: projectconfig.ComponentOverlayRemoveFile, + Archive: archiveName, + Filename: "remove-me.txt", + }, + }, + }) + + sourceManager.EXPECT().FetchFiles(gomock.Any(), component, outputDir).Return(nil) + sourceManager.EXPECT().FetchComponent(gomock.Any(), component, outputDir, gomock.Any()).DoAndReturn( + func(_ interface{}, _ interface{}, dir string, _ ...sourceproviders.FetchComponentOption) error { + return fileutils.WriteFile( + ctx.FS(), filepath.Join(dir, componentName+".spec"), + []byte("# test spec"), fileperms.PublicFile) + }, + ) + + preparer, err := sources.NewPreparer(sourceManager, ctx.FS(), ctx, ctx) + require.NoError(t, err) + + err = preparer.PrepareSources(ctx, component, outputDir, true /*applyOverlays*/) + require.Error(t, err, "preparing sources should fail when a modified archive has no 'sources' entry") + assert.Contains(t, err.Error(), archiveName, + "error should identify the archive missing from the 'sources' file") +} + func TestPrepareSources_SourceManagerError(t *testing.T) { ctrl := gomock.NewController(t) component := components_testutils.NewMockComponent(ctrl) @@ -854,7 +1028,7 @@ func TestPrepareSources_UpdatesSourcesFile(t *testing.T) { existingSourcesContent: "SHA512 (dup.tar.gz) = aaaa1111\nSHA512 (dup.tar.gz) = bbbb2222\n", expectError: true, errorContains: []string{ - "failed to parse existing 'sources' file", + "failed to parse 'sources' file", "duplicate filename", "dup.tar.gz", }, diff --git a/internal/fingerprint/fingerprint_test.go b/internal/fingerprint/fingerprint_test.go index 4a6cf1eb..6be16477 100644 --- a/internal/fingerprint/fingerprint_test.go +++ b/internal/fingerprint/fingerprint_test.go @@ -251,6 +251,32 @@ func TestComputeIdentity_OverlaySourceFileChange(t *testing.T) { assert.NotEqual(t, fp1, fp2, "different overlay source content must produce different fingerprints") } +func TestComputeIdentity_OverlayArchiveScopingChangesFP(t *testing.T) { + // Archive scoping comes from the overlay's file path, which is part of the hashed + // config, so scoping an overlay to an archive must change the fingerprint. Guards + // against excluding the path (e.g. `fingerprint:"-"`), which would let an archive + // overlay skip a rebuild. + ctx := newTestFS(t, map[string]string{ + "/specs/test.spec": "Name: testpkg\nVersion: 1.0", + }) + releaseVer := testReleaseVer + + base := baseComponent() + base.Overlays = []projectconfig.ComponentOverlay{ + {Type: "file-remove", Filename: "bundled.conf"}, + } + fpBase := computeFingerprint(t, ctx, base, releaseVer, 0) + + withArchive := baseComponent() + withArchive.Overlays = []projectconfig.ComponentOverlay{ + {Type: "file-remove", Archive: "pkg-1.0.tar.gz", Filename: "bundled.conf"}, + } + fpArchive := computeFingerprint(t, ctx, withArchive, releaseVer, 0) + + assert.NotEqual(t, fpBase, fpArchive, + "scoping the overlay to an archive (via the 'archive' field) must change the fingerprint") +} + func TestComputeIdentity_PatchAddRenameChangesFP(t *testing.T) { // When patch-add omits 'file', the destination filename is derived from // filepath.Base(Source). Renaming the source file changes the rendered diff --git a/internal/projectconfig/fingerprint_test.go b/internal/projectconfig/fingerprint_test.go index 9fe56870..7c8cc966 100644 --- a/internal/projectconfig/fingerprint_test.go +++ b/internal/projectconfig/fingerprint_test.go @@ -5,6 +5,7 @@ package projectconfig_test import ( "reflect" + "strings" "testing" "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" @@ -110,15 +111,29 @@ func TestAllFingerprintedFieldsHaveDecision(t *testing.T) { tag := field.Tag.Get("fingerprint") - switch tag { + // hashstructure tags are `name,option,...`; the name part decides + // inclusion ("-" excludes, anything else includes) and the options + // tune how an included field is hashed. + name, options, _ := strings.Cut(tag, ",") + + switch name { case "": - // No tag — included by default (the safe default). + // Empty name — included by default (the safe default). The only + // option we permit is `omitempty`, which makes hashstructure skip + // the field when it holds its zero value (so an unset field never + // perturbs the hash) while still hashing it when set. Reject any + // other option as a likely typo. + if options != "" && options != "omitempty" { + assert.Failf(t, "invalid fingerprint tag", + "field %q has unrecognised fingerprint tag option %q — "+ + "only `omitempty` is supported on included fields", key, options) + } case "-": actualExclusions[key] = true default: - // hashstructure only recognises "" (include) and "-" (exclude). - // Any other value is silently treated as included, which is - // almost certainly a typo. + // hashstructure only recognises "" (include) and "-" (exclude) + // for the name part. Any other value is silently treated as + // included, which is almost certainly a typo. assert.Failf(t, "invalid fingerprint tag", "field %q has unrecognised fingerprint tag value %q — "+ "only `fingerprint:\"-\"` (exclude) is valid; "+ diff --git a/internal/projectconfig/overlay.go b/internal/projectconfig/overlay.go index d890a6fe..731490d7 100644 --- a/internal/projectconfig/overlay.go +++ b/internal/projectconfig/overlay.go @@ -11,10 +11,13 @@ import ( "github.com/bmatcuk/doublestar/v4" "github.com/brunoga/deep" "github.com/microsoft/azure-linux-dev-tools/internal/global/opctx" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/archive" "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileutils" ) // ComponentOverlay represents an overlay that may be applied to a component's spec and/or its sources. +// +//nolint:recvcheck // HashInclude needs a value receiver for hashstructure; all other methods use pointer receivers. type ComponentOverlay struct { // The type of overlay to apply. Type ComponentOverlayType `toml:"type" json:"type" validate:"required" jsonschema:"enum=spec-add-tag,enum=spec-insert-tag,enum=spec-set-tag,enum=spec-update-tag,enum=spec-remove-tag,enum=spec-prepend-lines,enum=spec-append-lines,enum=spec-search-replace,enum=spec-remove-section,enum=spec-remove-subpackage,enum=patch-add,enum=patch-remove,enum=file-prepend-lines,enum=file-search-replace,enum=file-add,enum=file-remove,enum=file-rename,title=Overlay type,description=The type of overlay to apply"` @@ -23,7 +26,12 @@ type ComponentOverlay struct { // For overlays that apply to non-spec files, indicates the filename. For overlays that can // apply to multiple files, supports glob patterns (including globstar). - Filename string `toml:"file,omitempty" json:"file,omitempty" jsonschema:"title=Filename,description=The name of the non-spec file to which this overlay applies, or a glob pattern matching multiple files"` + Filename string `toml:"file,omitempty" json:"file,omitempty" jsonschema:"title=Filename,description=The name of the non-spec file to which this overlay applies. May be a glob pattern matching multiple files"` + // For archive-scoped overlays (file-remove and file-search-replace), the name of the source + // archive to extract, modify, and repack (e.g. "pkg-1.0.tar.gz"). When set, [ComponentOverlay.Filename] + // is interpreted as a glob relative to the archive's extraction root rather than a loose + // file in the sources tree. + Archive string `toml:"archive,omitempty" json:"archive,omitempty" jsonschema:"title=Archive,description=For archive-scoped overlays: the source archive to extract and repack (e.g. pkg-1.0.tar.gz). When set the 'file' field is a glob relative to the archive extraction root"` // For overlays that apply to specs, indicates the name of the section to which it applies. // Optional for spec-prepend-lines, spec-append-lines, and spec-search-replace: when omitted, // the overlay targets the entire spec file (prepend at top, append at end, search-replace @@ -131,10 +139,54 @@ func (c *ComponentOverlay) ModifiesSpec() bool { c.Type == ComponentOverlayRemovePatch } -// ModifiesNonSpecFiles returns true if the overlay modifies non-spec files. This includes -// hybrid overlays that modify both spec and source files (e.g., patch overlays), since -// those also require non-spec modifications. -func (c *ComponentOverlay) ModifiesNonSpecFiles() bool { +// ArchiveTarget returns the archive name and the inner-archive glob when the overlay +// is archive-scoped (i.e. [ComponentOverlay.Archive] is non-empty and +// [ComponentOverlay.Filename] provides the inner path). +// +// Returns ok=false when [ComponentOverlay.Archive] is empty, indicating a loose-file +// overlay whose [ComponentOverlay.Filename] refers to files in the sources tree directly. +func (c *ComponentOverlay) ArchiveTarget() (archiveName, innerPath string, ok bool) { + if c.Archive == "" || c.Filename == "" { + return "", "", false + } + + return c.Archive, c.Filename, true +} + +// SupportsArchiveScope reports whether the overlay type can target files inside a source +// archive via an archive-scoped [ComponentOverlay.Filename] (see [ComponentOverlay.ArchiveTarget]). +// Only file-remove and file-search-replace extract and repack archives; all other types treat +// the path as loose files in the sources tree. +func (c *ComponentOverlay) SupportsArchiveScope() bool { + return c.Type == ComponentOverlayRemoveFile || c.Type == ComponentOverlaySearchAndReplaceInFile +} + +// ModifiesArchive returns true if the overlay modifies files inside a source archive. +// These overlays require extraction and repacking of the archive. Only file-remove and +// file-search-replace support archive scoping (see [ComponentOverlay.SupportsArchiveScope]), +// and only when their [ComponentOverlay.Filename] is an archive-scoped path +// (see [ComponentOverlay.ArchiveTarget]). +func (c *ComponentOverlay) ModifiesArchive() bool { + if !c.SupportsArchiveScope() { + return false + } + + _, _, ok := c.ArchiveTarget() + + return ok +} + +// ModifiesLooseFiles returns true if the overlay modifies loose files in the +// sources tree (as opposed to the spec or files inside an archive). This includes +// hybrid overlays that modify both the spec and loose source files (e.g., patch +// overlays), since those also require loose-file modifications. Archive-scoped +// overlays (see [ModifiesArchive]) are excluded: they operate on files inside an +// archive, not loose files in the sources tree. +func (c *ComponentOverlay) ModifiesLooseFiles() bool { + if c.ModifiesArchive() { + return false + } + return c.Type == ComponentOverlayPrependLinesToFile || c.Type == ComponentOverlaySearchAndReplaceInFile || c.Type == ComponentOverlayAddFile || @@ -144,6 +196,26 @@ func (c *ComponentOverlay) ModifiesNonSpecFiles() bool { c.Type == ComponentOverlayRemovePatch } +// HashInclude implements the hashstructure Includable interface so the +// [ComponentOverlay.Archive] field is omitted from the component fingerprint while it holds +// its default (empty) value. A defaulted Archive therefore reproduces the pre-existing fingerprint +// (no cascading rebuild for existing overlays), while a non-empty Archive contributes to the hash. +// Every other field is always included, preserving existing hashing behaviour. +// +// NOTE: Remove this method once the RFC for explicit fingerprint exclusions is implemented and +// [ComponentOverlay.Archive] can be tagged with `fingerprint:"-,nonzero"` (or equivalent) instead. +// +// NOTE: the value receiver is required — [fingerprint.ComputeIdentity] hashes the component by +// value, so the struct (and its nested fields) are not addressable and a pointer-receiver method +// would never be detected by hashstructure. +func (c ComponentOverlay) HashInclude(field string, _ any) (bool, error) { + if field == "Archive" { + return c.Archive != "", nil + } + + return true, nil +} + // ComponentOverlayType is the type of a component overlay. type ComponentOverlayType string @@ -190,7 +262,10 @@ const ( ComponentOverlaySearchAndReplaceInFile ComponentOverlayType = "file-search-replace" // ComponentOverlayAddFile is an overlay that adds a non-spec file. ComponentOverlayAddFile ComponentOverlayType = "file-add" - // ComponentOverlayRemoveFile is an overlay that removes a non-spec file. + // ComponentOverlayRemoveFile is an overlay that removes a non-spec file. When + // [ComponentOverlay.Archive] is set, it removes file(s) matching the glob in + // [ComponentOverlay.Filename] from inside that source archive instead of loose + // files in the sources tree (see [ComponentOverlay.ArchiveTarget]). ComponentOverlayRemoveFile ComponentOverlayType = "file-remove" // ComponentOverlayRenameFile is an overlay that renames a non-spec file. ComponentOverlayRenameFile ComponentOverlayType = "file-rename" @@ -217,7 +292,41 @@ func (c *ComponentOverlay) Validate() error { return nil } +func (c *ComponentOverlay) validateArchiveField(desc string) error { + if c.Archive == "" { + return nil + } + + if !archive.IsArchiveName(c.Archive) { + return fmt.Errorf( + "overlay 'archive' field %#q is not a recognized archive name: %s", + c.Archive, desc, + ) + } + + if err := fileutils.ValidateFilename(c.Archive); err != nil { + return fmt.Errorf("unsafe overlay 'archive' field %#q: %s:\n%w", c.Archive, desc, err) + } + + if !c.SupportsArchiveScope() { + return fmt.Errorf( + "overlay type %#q does not support archive-scoped file paths: %s", + c.Type, desc, + ) + } + + if c.Filename == "" { + return fmt.Errorf("overlay type %#q requires %#q field: %s", c.Type, "file", desc) + } + + return nil +} + func (c *ComponentOverlay) validateRequiredFields(desc string) error { + if err := c.validateArchiveField(desc); err != nil { + return err + } + switch c.Type { case ComponentOverlayAddSpecTag, ComponentOverlayInsertSpecTag, ComponentOverlaySetSpecTag, ComponentOverlayUpdateSpecTag, ComponentOverlayRemoveSpecTag: diff --git a/internal/projectconfig/overlay_test.go b/internal/projectconfig/overlay_test.go index ece2252e..1313562f 100644 --- a/internal/projectconfig/overlay_test.go +++ b/internal/projectconfig/overlay_test.go @@ -319,6 +319,78 @@ func TestComponentOverlay_Validate(t *testing.T) { errorExpected: true, errorContains: "source", }, + // Archive-scoped validation: only file-remove and file-search-replace support 'archive' field. + { + name: "file-remove archive-scoped path accepted", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayRemoveFile, + Archive: "pkg-1.0.tar.gz", + Filename: "vendor/**", + }, + errorExpected: false, + }, + { + name: "file-search-replace archive-scoped path accepted", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlaySearchAndReplaceInFile, + Archive: "pkg-1.0.tar.gz", + Filename: "vendor/config.h", + Regex: "old", + Replacement: "new", + }, + errorExpected: false, + }, + { + name: "file-add archive field rejected", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayAddFile, + Archive: "pkg-1.0.tar.gz", + Filename: "vendor/new.txt", + Source: "/path/to/source.txt", + }, + errorExpected: true, + errorContains: "archive-scoped", + }, + { + name: "file-prepend-lines archive field rejected", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayPrependLinesToFile, + Archive: "pkg-1.0.tar.gz", + Filename: "vendor/config.h", + Lines: []string{"// header"}, + }, + errorExpected: true, + errorContains: "archive-scoped", + }, + { + name: "patch-remove archive field rejected", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayRemovePatch, + Archive: "pkg-1.0.tar.gz", + Filename: "fix.patch", + }, + errorExpected: true, + errorContains: "archive-scoped", + }, + { + name: "archive field with unrecognized extension rejected", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayRemoveFile, + Archive: "pkg-1.0.zip", + Filename: "vendor/**", + }, + errorExpected: true, + errorContains: "not a recognized archive name", + }, + { + name: "archive field without file field rejected", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayRemoveFile, + Archive: "pkg-1.0.tar.gz", + }, + errorExpected: true, + errorContains: "file", + }, // Description included in error { name: "error includes description", @@ -469,6 +541,53 @@ func TestComponentOverlay_Validate(t *testing.T) { errorExpected: true, errorContains: "section", }, + // archive-scoped file-remove tests (explicit 'archive' field) + { + name: "file-remove archive-scoped valid", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayRemoveFile, + Archive: "pkg-1.0.tar.gz", + Filename: "unwanted.conf", + }, + errorExpected: false, + }, + { + name: "file-remove archive-scoped glob valid", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayRemoveFile, + Archive: "pkg-1.0.tar.gz", + Filename: "docs/**/*.md", + }, + errorExpected: false, + }, + { + name: "file-remove of a bare archive name is a plain loose-file remove", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayRemoveFile, + Filename: "old.tar.gz", + }, + errorExpected: false, + }, + { + name: "file-remove without archive field is a plain loose-file remove", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayRemoveFile, + Filename: "unwanted.conf", + }, + errorExpected: false, + }, + // file-search-replace supports archive scoping via the explicit 'archive' field + { + name: "file-search-replace archive-scoped valid", + overlay: projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlaySearchAndReplaceInFile, + Archive: "pkg-1.0.tar.gz", + Filename: "config.h", + Regex: "old_value", + Replacement: "new_value", + }, + errorExpected: false, + }, } for _, testCase := range testCases { @@ -512,6 +631,16 @@ func TestComponentOverlay_ModifiesSpec(t *testing.T) { projectconfig.ComponentOverlayAddFile, } + // Archive-scoped overlays: only file-remove/file-search-replace become archive-scoped, + // and only when [ComponentOverlay.Archive] is set. + archiveOverlays := []projectconfig.ComponentOverlay{ + {Type: projectconfig.ComponentOverlayRemoveFile, Archive: "pkg-1.0.tar.gz", Filename: "f"}, + { + Type: projectconfig.ComponentOverlaySearchAndReplaceInFile, + Archive: "pkg-1.0.tar.gz", Filename: "f", Regex: "old", Replacement: "new", + }, + } + for _, overlayType := range specOverlayTypes { t.Run(string(overlayType)+"_is_spec_overlay", func(t *testing.T) { overlay := projectconfig.ComponentOverlay{Type: overlayType} @@ -525,4 +654,13 @@ func TestComponentOverlay_ModifiesSpec(t *testing.T) { assert.False(t, overlay.ModifiesSpec(), "expected %s to not be a spec overlay", overlayType) }) } + + for _, overlay := range archiveOverlays { + t.Run(string(overlay.Type)+"_is_archive_scoped", func(t *testing.T) { + assert.True(t, overlay.ModifiesArchive(), "expected %s to be an archive-scoped overlay", overlay.Type) + assert.False(t, overlay.ModifiesSpec(), "expected %s to not be a spec overlay", overlay.Type) + assert.False(t, overlay.ModifiesLooseFiles(), + "expected %s to not be a loose-file overlay", overlay.Type) + }) + } } diff --git a/internal/utils/archive/archive.go b/internal/utils/archive/archive.go index 65be9a18..62699d76 100644 --- a/internal/utils/archive/archive.go +++ b/internal/utils/archive/archive.go @@ -11,6 +11,11 @@ // are not policed by [os.Root]; this package additionally rejects any symlink // whose target is non-local via [filepath.IsLocal]. // +// Only regular files, directories, and symlinks are extracted. Other entry +// types (hardlinks, devices, FIFOs, etc.) are skipped by default, or cause a +// failure when [WithErrorOnUnsupportedEntry] is set — callers that repack the +// tree must set it so such entries aren't silently dropped. +// // Archive creation is designed for reproducible builds: file ordering is // lexicographic, timestamps are pinned to Unix epoch, and owner/group metadata // is zeroed out. This matches the @@ -20,6 +25,8 @@ package archive import ( "archive/tar" + "bufio" + "bytes" "compress/gzip" "errors" "fmt" @@ -50,6 +57,11 @@ const ( CompressionZstd ) +// compressionMagicLen is the number of leading bytes [sniffCompression] needs +// to recognize every supported compressed format (xz has the longest, 6-byte, +// signature). +const compressionMagicLen = 6 + // maxEntryBytes caps the decompressed size of any single regular-file entry // extracted by [Extract]. This prevents a decompression-bomb archive from // filling the destination filesystem. 10 GiB is well above any reasonable @@ -79,13 +91,65 @@ func DetectCompression(filename string) (Compression, error) { } } +// IsArchiveName reports whether filename has a recognized archive extension. +// It is a convenience predicate over [DetectCompression] for classifying a path +// as an archive without needing the specific compression type. +func IsArchiveName(filename string) bool { + _, err := DetectCompression(filename) + + return err == nil +} + +// ExtractAuto is a convenience wrapper that infers the compression from +// archivePath's extension via [DetectCompression] and then calls [Extract]. +// Most callers should prefer this over the explicit-compression [Extract], +// which exists for cases where the compression cannot be derived from the +// filename. +func ExtractAuto(archivePath, destDir string, opts ...ExtractOption) error { + comp, err := DetectCompression(archivePath) + if err != nil { + return fmt.Errorf("detecting compression for %#q:\n%w", archivePath, err) + } + + return Extract(archivePath, destDir, comp, opts...) +} + +// ExtractOption configures the behavior of [Extract] and [ExtractAuto]. +type ExtractOption func(*extractConfig) + +// extractConfig holds the resolved options for an extraction. +type extractConfig struct { + errorOnUnsupportedEntry bool +} + +// WithErrorOnUnsupportedEntry makes [Extract] fail on tar entries it cannot +// materialize (anything but a regular file, directory, or symlink). Without it +// such entries are skipped; callers that repack the tree should set it so the +// entries aren't silently dropped from the rebuilt archive. +func WithErrorOnUnsupportedEntry() ExtractOption { + return func(c *extractConfig) { + c.errorOnUnsupportedEntry = true + } +} + // Extract reads a tar archive, decompresses it, and extracts all entries into -// destDir. Supported entry types are regular files, directories, and symlinks; -// other entry types are skipped. Entry paths are confined to destDir via -// [os.Root]: any path that would escape destDir is rejected by the runtime. -// Symlink targets are validated separately by this package — see the package -// doc for details. -func Extract(archivePath, destDir string, comp Compression) (err error) { +// destDir. Entry paths are confined to destDir via [os.Root]: any path that +// would escape destDir is rejected by the runtime. Symlink targets are +// validated separately by this package — see the package doc for details. +// +// Only regular files, directories, and symlinks are supported. Other entry +// types are skipped by default, or fail when [WithErrorOnUnsupportedEntry] is +// set (required by callers that repack the tree). +func Extract(archivePath, destDir string, comp Compression, opts ...ExtractOption) (err error) { + var cfg extractConfig + for _, opt := range opts { + opt(&cfg) + } + + if comp < CompressionNone || comp > CompressionZstd { + return fmt.Errorf("unsupported compression type %d", comp) + } + if err := os.MkdirAll(destDir, fileperms.PublicDir); err != nil { return fmt.Errorf("creating destination %#q:\n%w", destDir, err) } @@ -102,7 +166,13 @@ func Extract(archivePath, destDir string, comp Compression) (err error) { } defer defers.HandleDeferError(file.Close, &err) - decompressed, closer, err := newDecompressor(file, comp) + // Prefer the actual compression detected from the leading magic bytes over + // the caller-supplied (extension-derived) value: upstream archives are + // sometimes mislabeled, e.g. an uncompressed tar published as ".txz". + bufReader := bufio.NewReader(file) + magic, _ := bufReader.Peek(compressionMagicLen) + + decompressed, closer, err := newDecompressor(bufReader, sniffCompression(magic, comp)) if err != nil { return err } @@ -123,12 +193,57 @@ func Extract(archivePath, destDir string, comp Compression) (err error) { return fmt.Errorf("reading tar entry from %#q:\n%w", archivePath, readErr) } - if err := extractEntry(root, header, tarReader); err != nil { + if err := extractEntry(root, header, tarReader, cfg); err != nil { return fmt.Errorf("extracting %#q from %#q:\n%w", header.Name, archivePath, err) } } } +// sniffCompression returns the compression format implied by the leading magic +// bytes, falling back to fallback when no known signature matches (an +// uncompressed tar carries no leading magic). All supported compressed formats +// have a fixed-position header, so this is authoritative over the filename +// extension. +func sniffCompression(magic []byte, fallback Compression) Compression { + switch { + case bytes.HasPrefix(magic, []byte{0xFD, '7', 'z', 'X', 'Z', 0x00}): + return CompressionXZ + case bytes.HasPrefix(magic, []byte{0x1F, 0x8B}): + return CompressionGzip + case bytes.HasPrefix(magic, []byte{0x28, 0xB5, 0x2F, 0xFD}): + return CompressionZstd + case len(magic) == 0: + // Unreadable/empty peek: defer to the extension-derived hint. + return fallback + default: + // No compression signature: an uncompressed tar (whatever the + // extension claims). A real tar's "ustar" magic sits at byte 257. + return CompressionNone + } +} + +// SniffCompressionFromFile determines an existing archive's compression by +// inspecting its leading magic bytes — authoritative over the filename +// extension. +func SniffCompressionFromFile(archivePath string, fallback Compression) (comp Compression, err error) { + file, err := os.Open(archivePath) + if err != nil { + return fallback, fmt.Errorf("opening %#q for compression sniffing:\n%w", archivePath, err) + } + defer defers.HandleDeferError(file.Close, &err) + + magic := make([]byte, compressionMagicLen) + + bytesRead, readErr := io.ReadFull(file, magic) + // Short files are fine — a tiny tar may be under compressionMagicLen bytes. + // Only a genuine read error (not EOF/short read) is fatal. + if readErr != nil && !errors.Is(readErr, io.EOF) && !errors.Is(readErr, io.ErrUnexpectedEOF) { + return fallback, fmt.Errorf("reading %#q header:\n%w", archivePath, readErr) + } + + return sniffCompression(magic[:bytesRead], fallback), nil +} + // newDecompressor wraps reader in the chosen decompressor. For // [CompressionNone] the returned closer is nil; otherwise it is the // decompressor itself. @@ -235,9 +350,13 @@ func deterministicEpoch() time.Time { return time.Unix(0, 0).UTC() } -func extractEntry(root *os.Root, header *tar.Header, tarReader io.Reader) error { +func extractEntry(root *os.Root, header *tar.Header, tarReader io.Reader, cfg extractConfig) error { name := header.Name + if header.Typeflag == tar.TypeXGlobalHeader || header.Typeflag == tar.TypeXHeader { + return nil + } + if header.Typeflag == tar.TypeDir { if err := root.MkdirAll(name, fileperms.PublicDir); err != nil { return fmt.Errorf("creating directory %#q:\n%w", name, err) @@ -272,6 +391,14 @@ func extractEntry(root *os.Root, header *tar.Header, tarReader io.Reader) error case tar.TypeReg: return extractRegularFile(root, header, tarReader) default: + // Unsupported entry type (hardlink, device, FIFO, ...): fail when the + // caller is strict (repacking would drop it), otherwise skip and log. + if cfg.errorOnUnsupportedEntry { + return fmt.Errorf( + "tar entry %#q has unsupported type (typeflag %d); only regular files, directories, and symlinks are supported", + name, header.Typeflag) + } + slog.Debug("Skipping unsupported tar entry type", "name", name, "typeflag", header.Typeflag) return nil diff --git a/internal/utils/archive/archive_test.go b/internal/utils/archive/archive_test.go index fece1cd0..082e0b03 100644 --- a/internal/utils/archive/archive_test.go +++ b/internal/utils/archive/archive_test.go @@ -52,6 +52,63 @@ func TestDetectCompression(t *testing.T) { } } +// TestSniffCompressionFromFile verifies that compression is detected from the +// file's real magic bytes, ignoring (and overriding) the filename extension. +func TestSniffCompressionFromFile(t *testing.T) { + tmpDir := t.TempDir() + + // Write a real archive of each format under a deliberately misleading ".txz" + // name, then assert the sniffer reports the real format, not what the name claims. + write := func(t *testing.T, name string, comp archive.Compression) string { + t.Helper() + + src := filepath.Join(t.TempDir(), "src") + require.NoError(t, os.MkdirAll(src, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(src, "a.txt"), []byte("hi"), 0o600)) + + path := filepath.Join(tmpDir, name) + require.NoError(t, archive.CreateDeterministicArchive(path, src, comp)) + + return path + } + + tests := []struct { + name string + comp archive.Compression + }{ + {"plain-tar-named.txz", archive.CompressionNone}, + {"gzip-named.txz", archive.CompressionGzip}, + {"xz-named.tar", archive.CompressionXZ}, + {"zstd-named.tgz", archive.CompressionZstd}, + } + + for _, testCase := range tests { + t.Run(testCase.name, func(t *testing.T) { + path := write(t, testCase.name, testCase.comp) + + // Fallback is deliberately a wrong value to prove it is never used + // for a non-empty file. + got, err := archive.SniffCompressionFromFile(path, archive.CompressionGzip) + require.NoError(t, err) + assert.Equal(t, testCase.comp, got) + }) + } + + t.Run("empty file uses fallback", func(t *testing.T) { + path := filepath.Join(tmpDir, "empty.txz") + require.NoError(t, os.WriteFile(path, nil, 0o600)) + + got, err := archive.SniffCompressionFromFile(path, archive.CompressionXZ) + require.NoError(t, err) + assert.Equal(t, archive.CompressionXZ, got) + }) + + t.Run("missing file errors", func(t *testing.T) { + _, err := archive.SniffCompressionFromFile(filepath.Join(tmpDir, "nope.tar"), archive.CompressionNone) + require.Error(t, err) + }) +} + func TestExtractAndRepack(t *testing.T) { tmpDir := t.TempDir() @@ -121,6 +178,8 @@ func createTestTarGz(t *testing.T, path string, entries []testTarEntry) { header.Size = int64(len(entry.content)) case tar.TypeSymlink: header.Linkname = entry.linkname + case tar.TypeLink: + header.Linkname = entry.linkname } require.NoError(t, tarWriter.WriteHeader(header)) @@ -187,14 +246,61 @@ func TestRoundTrip_AllCompressions(t *testing.T) { } } +func TestExtract_MislabeledCompression(t *testing.T) { + // Some upstream archives are published with a compression extension that + // doesn't match their bytes (e.g. an uncompressed tar named ".txz"). Extract + // must sniff the real format from the magic bytes rather than trusting the + // caller-supplied (extension-derived) compression. + tests := []struct { + name string + // actual is the format the bytes are really written in. + actual archive.Compression + // claimed is the (wrong) compression Extract is told to use. + claimed archive.Compression + }{ + {"plain tar claimed as xz", archive.CompressionNone, archive.CompressionXZ}, + {"plain tar claimed as gzip", archive.CompressionNone, archive.CompressionGzip}, + {"gzip claimed as xz", archive.CompressionGzip, archive.CompressionXZ}, + {"xz claimed as none", archive.CompressionXZ, archive.CompressionNone}, + {"zstd claimed as gzip", archive.CompressionZstd, archive.CompressionGzip}, + } + + for _, testCase := range tests { + t.Run(testCase.name, func(t *testing.T) { + tmpDir := t.TempDir() + sourceDir := filepath.Join(tmpDir, "src") + extractDir := filepath.Join(tmpDir, "out") + + require.NoError(t, os.MkdirAll(sourceDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(sourceDir, "a.txt"), []byte("alpha"), 0o600)) + + // Write the archive in its real format but with a mismatched name. + archivePath := filepath.Join(tmpDir, "pkg.txz") + require.NoError(t, archive.CreateDeterministicArchive(archivePath, sourceDir, testCase.actual)) + + // Extract is given the wrong compression; magic-byte sniffing must + // recover the real format and extract successfully. + require.NoError(t, archive.Extract(archivePath, extractDir, testCase.claimed)) + + got, err := os.ReadFile(filepath.Join(extractDir, "a.txt")) + require.NoError(t, err) + assert.Equal(t, "alpha", string(got)) + }) + } +} + func TestUnsupportedCompression(t *testing.T) { tmpDir := t.TempDir() - archivePath := filepath.Join(tmpDir, "archive.bin") - require.NoError(t, os.WriteFile(archivePath, []byte("dummy"), 0o600)) bogus := archive.Compression(99) - err := archive.Extract(archivePath, tmpDir, bogus) + // Extract sniffs real compression from magic bytes and only falls back to + // the caller-supplied value when there are no bytes to inspect. Use an empty + // file so the bogus value actually reaches the decompressor guard. + emptyPath := filepath.Join(tmpDir, "empty.bin") + require.NoError(t, os.WriteFile(emptyPath, nil, 0o600)) + + err := archive.Extract(emptyPath, tmpDir, bogus) require.Error(t, err) assert.Contains(t, err.Error(), "unsupported compression type") @@ -203,6 +309,103 @@ func TestUnsupportedCompression(t *testing.T) { assert.Contains(t, err.Error(), "unsupported compression type") } +// TestExtract_UnsupportedEntryType verifies the handling of tar entries whose +// type cannot be materialized (here, a hardlink). By default such entries are +// skipped; with [archive.WithErrorOnUnsupportedEntry] they cause a failure so +// callers that repack the tree don't silently drop them. +func TestExtract_UnsupportedEntryType(t *testing.T) { + makeArchive := func(t *testing.T) string { + t.Helper() + + archivePath := filepath.Join(t.TempDir(), "pkg.tar.gz") + createTestTarGz(t, archivePath, []testTarEntry{ + {name: "pkg/real.txt", typeflag: tar.TypeReg, content: "hello"}, + // A hardlink to the regular file above: a valid tar entry type that + // Extract cannot materialize. + {name: "pkg/link.txt", typeflag: tar.TypeLink, linkname: "pkg/real.txt"}, + }) + + return archivePath + } + + t.Run("skipped by default", func(t *testing.T) { + extractDir := filepath.Join(t.TempDir(), "out") + + require.NoError(t, archive.Extract(makeArchive(t), extractDir, archive.CompressionGzip)) + + // The regular file is extracted; the unsupported hardlink is skipped. + assert.FileExists(t, filepath.Join(extractDir, "pkg", "real.txt")) + assert.NoFileExists(t, filepath.Join(extractDir, "pkg", "link.txt")) + }) + + t.Run("errors with WithErrorOnUnsupportedEntry", func(t *testing.T) { + extractDir := filepath.Join(t.TempDir(), "out") + + err := archive.Extract( + makeArchive(t), extractDir, archive.CompressionGzip, + archive.WithErrorOnUnsupportedEntry(), + ) + require.Error(t, err) + assert.Contains(t, err.Error(), "unsupported type") + assert.Contains(t, err.Error(), "link.txt") + }) +} + +// TestExtract_PaxGlobalHeaderIgnored is a regression test: git-generated +// tarballs (e.g. GitHub archives) begin with a "pax_global_header" entry +// (typeflag 'g'). It is tar metadata, not file content, so it must be skipped +// even under [archive.WithErrorOnUnsupportedEntry] — otherwise repack-strict +// callers cannot extract any git-generated source archive. +func TestExtract_PaxGlobalHeaderIgnored(t *testing.T) { + archivePath := filepath.Join(t.TempDir(), "pkg.tar.gz") + + file, err := os.Create(archivePath) + require.NoError(t, err) + + gzWriter := gzip.NewWriter(file) + tarWriter := tar.NewWriter(gzWriter) + + // Leading PAX global header carrying a commit hash, exactly as `git archive` + // emits. Go names it "pax_global_header" on read-back. + require.NoError(t, tarWriter.WriteHeader(&tar.Header{ + Typeflag: tar.TypeXGlobalHeader, + Name: "pax_global_header", + PAXRecords: map[string]string{ + "comment": "0123456789abcdef0123456789abcdef01234567", + }, + })) + + const fileContent = "hello" + + require.NoError(t, tarWriter.WriteHeader(&tar.Header{ + Typeflag: tar.TypeReg, + Name: "pkg-1.0/real.txt", + Mode: 0o644, + Size: int64(len(fileContent)), + })) + _, err = tarWriter.Write([]byte(fileContent)) + require.NoError(t, err) + + require.NoError(t, tarWriter.Close()) + require.NoError(t, gzWriter.Close()) + require.NoError(t, file.Close()) + + extractDir := filepath.Join(t.TempDir(), "out") + + // Strict mode must succeed: the global header is skipped, the file extracted. + require.NoError(t, archive.Extract( + archivePath, extractDir, archive.CompressionGzip, + archive.WithErrorOnUnsupportedEntry(), + )) + + content, err := os.ReadFile(filepath.Join(extractDir, "pkg-1.0", "real.txt")) + require.NoError(t, err) + assert.Equal(t, fileContent, string(content)) + + // The pax header must not have produced a file on disk. + assert.NoFileExists(t, filepath.Join(extractDir, "pax_global_header")) +} + func TestCreateDeterministicArchive_PreservesSymlinks(t *testing.T) { tmpDir := t.TempDir() sourceDir := filepath.Join(tmpDir, "src") diff --git a/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap b/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap index 163b4fee..6ad50981 100755 --- a/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap +++ b/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap @@ -258,7 +258,12 @@ "file": { "type": "string", "title": "Filename", - "description": "The name of the non-spec file to which this overlay applies" + "description": "The name of the non-spec file to which this overlay applies. May be a glob pattern matching multiple files" + }, + "archive": { + "type": "string", + "title": "Archive", + "description": "For archive-scoped overlays: the source archive to extract and repack (e.g. pkg-1.0.tar.gz). When set the 'file' field is a glob relative to the archive extraction root" }, "section": { "type": "string", diff --git a/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap b/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap index 163b4fee..6ad50981 100755 --- a/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap +++ b/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap @@ -258,7 +258,12 @@ "file": { "type": "string", "title": "Filename", - "description": "The name of the non-spec file to which this overlay applies" + "description": "The name of the non-spec file to which this overlay applies. May be a glob pattern matching multiple files" + }, + "archive": { + "type": "string", + "title": "Archive", + "description": "For archive-scoped overlays: the source archive to extract and repack (e.g. pkg-1.0.tar.gz). When set the 'file' field is a glob relative to the archive extraction root" }, "section": { "type": "string", diff --git a/schemas/azldev.schema.json b/schemas/azldev.schema.json index 163b4fee..6ad50981 100644 --- a/schemas/azldev.schema.json +++ b/schemas/azldev.schema.json @@ -258,7 +258,12 @@ "file": { "type": "string", "title": "Filename", - "description": "The name of the non-spec file to which this overlay applies" + "description": "The name of the non-spec file to which this overlay applies. May be a glob pattern matching multiple files" + }, + "archive": { + "type": "string", + "title": "Archive", + "description": "For archive-scoped overlays: the source archive to extract and repack (e.g. pkg-1.0.tar.gz). When set the 'file' field is a glob relative to the archive extraction root" }, "section": { "type": "string",