diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f8783c..e594667 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `FormatBytes` now returns a sign-prefixed string for negative inputs instead of silently wrapping to a large positive value via an unchecked `uint64` cast. - Daemon file-viewer (`DockerExtractor.Extract`) now correctly marks the returned `FileContent` as truncated when the extracted file exceeds the 1 MB view limit. Previously the flag was always false for daemon-extracted files, suppressing the truncation notice in the viewer. - `ErrNoEngineFound.Cause` field and its `Unwrap()` method removed; neither construction site ever populated the field, so `errors.Unwrap` always returned `nil`, making the method misleading. +- Archive analysis now rejects an image archive where a manifest-referenced layer blob is absent from the outer tar. Previously the missing layer was silently treated as empty, which could cause CI rules to pass on incomplete input. +- Directory metadata (mode, UID, GID) set by an earlier layer is no longer overwritten by an implicit parent directory node created when a later layer adds a child without including an explicit directory header. The placeholder node is now marked inferred and skipped during metadata merging. +- Hardlink-aliased content is no longer counted as wasted bytes when the original filename is deleted but a hardlink pointing at the same payload survives in the final image. Previously the deleted name's bytes were charged as waste even though the payload remained reachable. ## [v1.6.1] - 2026-08-08 diff --git a/image/docker.go b/image/docker.go index b25e9fe..f560b2a 100644 --- a/image/docker.go +++ b/image/docker.go @@ -658,6 +658,13 @@ func parseLayers(ctx context.Context, r io.Reader) ([]Layer, error) { layers[idx].Tree = tree } + for i, layerPath := range manifest.Layers { + if _, present := headers[layerPath]; !present { + return nil, fmt.Errorf("layer %d (%s) is referenced in the manifest but absent from the archive", + i, layers[i].ID) + } + } + return layers, nil } diff --git a/image/docker_test.go b/image/docker_test.go index dd1c245..136c96a 100644 --- a/image/docker_test.go +++ b/image/docker_test.go @@ -964,3 +964,31 @@ func TestEnsureImage_PlatformMissing_ContainerdStore_NotMisclassifiedAsImageNotF // pinning to exactly one inspect locks the win in. assert.Equal(t, 1, inspectCalls, "platform classifier should reach enumeratePlatforms directly, no probe inspect needed") } + +func TestParseLayers_MissingLayerBlob_ReturnsError(t *testing.T) { + manifest := []dockerManifest{{ + Config: "config.json", + Layers: []string{"layer0/layer.tar", "layer1/layer.tar"}, + }} + manifestData, err := json.Marshal(manifest) + require.NoError(t, err) + + configData := buildConfig(t, []string{"RUN step0", "RUN step1"}) + + // Build a valid (empty) tar for layer0 so parseLayers succeeds Pass 2 for it; + // layer1 is absent entirely, which is what we want the new check to catch. + var layer0Buf bytes.Buffer + require.NoError(t, tar.NewWriter(&layer0Buf).Close()) + + // Only layer0 is present; layer1 is absent from the archive. + tarBuf := buildTar(t, map[string][]byte{ + "manifest.json": manifestData, + "config.json": configData, + "layer0/layer.tar": layer0Buf.Bytes(), + }) + + _, err = parseLayers(context.Background(), tarBuf) + require.Error(t, err) + assert.Contains(t, err.Error(), "layer 1") + assert.Contains(t, err.Error(), "absent from the archive") +} diff --git a/image/efficiency.go b/image/efficiency.go index 4839088..4767f1a 100644 --- a/image/efficiency.go +++ b/image/efficiency.go @@ -1,6 +1,9 @@ package image -import "sort" +import ( + "sort" + "strings" +) type WastedFile struct { Path string @@ -71,8 +74,8 @@ type pathRun struct { func computeEfficiency(layers []Layer, stacked []*FileTree) *EfficiencyResult { // Build a path→FileNode index per stacked snapshot once. pathRuns then does // O(1) lookups instead of recursing through the tree once per (path, - // snapshot) pair, which restored the analysis from quadratic to linear in - // total file count for layered images with many shared paths. + // snapshot) pair, which keeps the analysis linear in total file count for + // layered images with many shared paths. indices := make([]map[string]*FileNode, len(stacked)) for i, tree := range stacked { if tree == nil || tree.Root == nil { @@ -83,6 +86,15 @@ func computeEfficiency(layers []Layer, stacked []*FileTree) *EfficiencyResult { indices[i] = idx } + // Build the set of Linkname values for hardlinks that survive in the final + // stacked snapshot. A deleted path whose content is still reachable via a + // surviving hardlink alias is not truly wasted — the payload stays in the + // image and can be read through the alias. + survivingHardlinkTargets := make(map[string]struct{}) + if n := len(stacked); n > 0 && stacked[n-1] != nil && stacked[n-1].Root != nil { + walkSurvivingHardlinks(stacked[n-1].Root, survivingHardlinkTargets) + } + paths := make(map[string]struct{}) for _, layer := range layers { if layer.Tree == nil || layer.Tree.Root == nil { @@ -115,6 +127,10 @@ func computeEfficiency(layers []Layer, stacked []*FileTree) *EfficiencyResult { continue } charged = run.occ[:len(run.occ)-1] + } else if _, alive := survivingHardlinkTargets[path]; alive { + // The path was deleted, but a hardlink alias pointing at it + // survives in the final tree — the payload is still live. + continue } for _, occ := range charged { pathWaste += occ.size @@ -288,3 +304,19 @@ func walkFiles(node *FileNode, fn func(path string, size int64)) { } } } + +// walkSurvivingHardlinks collects the Linkname (target path) of every +// non-removed hardlink in the tree into targets. Used to avoid charging +// deleted original paths as waste when a hardlink alias still references them. +func walkSurvivingHardlinks(node *FileNode, targets map[string]struct{}) { + for _, child := range node.Children { + if isWhiteoutName(child.Name) || child.DiffType == Removed { + continue + } + if child.IsDir { + walkSurvivingHardlinks(child, targets) + } else if child.IsHardlink && child.Linkname != "" { + targets["/"+strings.TrimPrefix(child.Linkname, "/")] = struct{}{} + } + } +} diff --git a/image/efficiency_test.go b/image/efficiency_test.go index b334c65..93ef57e 100644 --- a/image/efficiency_test.go +++ b/image/efficiency_test.go @@ -429,3 +429,38 @@ func TestEfficiency_Golden(t *testing.T) { assert.Equal(t, int64(300), result.WastedFiles[1].TotalWasted) assert.Equal(t, 2, result.WastedFiles[1].LayerCount) } + +// Deleting the original filename of a hardlink pair must not be counted as +// waste when a surviving alias still holds the payload in the final image. +func TestEfficiency_HardlinkAlias_DeletedOriginal_NotWasted(t *testing.T) { + // Layer 0: /data/a is a 1 MiB regular file; /data/b is a hardlink → data/a. + hardlink := &FileNode{ + Name: "b", + Path: "/data/b", + Size: 0, + IsHardlink: true, + Linkname: "data/a", + } + layer0 := makeTree( + makeDir("data", "/data", + makeFile("a", "/data/a", 1<<20), + hardlink, + ), + ) + // Layer 1: .wh.a deletes /data/a; /data/b (the alias) survives. + layer1 := makeTree( + makeDir("data", "/data", + makeFile(".wh.a", "/data/.wh.a", 0), + ), + ) + + layers := []Layer{ + {Index: 0, Tree: layer0}, + {Index: 1, Tree: layer1}, + } + result := Efficiency(layers) + assert.Equal(t, int64(0), result.WastedBytes, + "payload is still reachable via /data/b — must not be charged as waste") + assert.Empty(t, result.WastedFiles) + assert.Equal(t, 1.0, result.Score) +} diff --git a/image/filetree.go b/image/filetree.go index b47f97b..1badbfa 100644 --- a/image/filetree.go +++ b/image/filetree.go @@ -33,6 +33,10 @@ type FileNode struct { Children []*FileNode IsDir bool IsHardlink bool + // Inferred is true for directory nodes created implicitly by insertNode + // because no explicit tar header existed for them. Their Mode/UID/GID are + // structural defaults, not authoritative image metadata. + Inferred bool } func NewFileTree() *FileTree { diff --git a/image/stack.go b/image/stack.go index 54468d7..3cd2e5c 100644 --- a/image/stack.go +++ b/image/stack.go @@ -66,9 +66,9 @@ func mergeLayerWith(cumulative, layerRoot *FileNode, layerIdx int, carry carryFo IntroducedInLayer: cumulative.IntroducedInLayer, } - metadataChanged := cumulative.Mode != layerRoot.Mode || + metadataChanged := !layerRoot.Inferred && (cumulative.Mode != layerRoot.Mode || cumulative.UID != layerRoot.UID || - cumulative.GID != layerRoot.GID + cumulative.GID != layerRoot.GID) if metadataChanged { merged.Mode = layerRoot.Mode merged.UID = layerRoot.UID @@ -222,6 +222,7 @@ func cloneAsUnchanged(node *FileNode) *FileNode { Size: node.Size, IsDir: node.IsDir, IsHardlink: node.IsHardlink, + Inferred: node.Inferred, DiffType: Unchanged, Mode: node.Mode, UID: node.UID, @@ -242,6 +243,7 @@ func cloneAsRemoved(node *FileNode) *FileNode { Size: node.Size, IsDir: node.IsDir, IsHardlink: node.IsHardlink, + Inferred: node.Inferred, DiffType: Removed, Mode: node.Mode, UID: node.UID, @@ -262,6 +264,7 @@ func cloneAsAdded(node *FileNode, layerIdx int) *FileNode { Size: node.Size, IsDir: node.IsDir, IsHardlink: node.IsHardlink, + Inferred: node.Inferred, DiffType: Added, Mode: node.Mode, UID: node.UID, @@ -287,6 +290,7 @@ func cloneStructure(node *FileNode) *FileNode { Size: node.Size, IsDir: node.IsDir, IsHardlink: node.IsHardlink, + Inferred: node.Inferred, Mode: node.Mode, UID: node.UID, GID: node.GID, @@ -314,6 +318,7 @@ func cloneWithDiffType(node *FileNode) *FileNode { Size: node.Size, IsDir: node.IsDir, IsHardlink: node.IsHardlink, + Inferred: node.Inferred, DiffType: node.DiffType, Mode: node.Mode, UID: node.UID, diff --git a/image/stack_test.go b/image/stack_test.go index 21453a8..9aa7388 100644 --- a/image/stack_test.go +++ b/image/stack_test.go @@ -1,6 +1,8 @@ package image import ( + "archive/tar" + "bytes" "io/fs" "testing" @@ -954,3 +956,51 @@ func TestBuildAggregatedTrees_EmptyLayerSnapshotsBaseline(t *testing.T) { require.NotNil(t, b2) assert.Equal(t, Added, b2.DiffType) } + +// An implicit parent directory node must not overwrite the real metadata an +// earlier layer recorded for that directory. +func TestStack_ImplicitParentDoesNotOverwriteMetadata(t *testing.T) { + // Layer 0: explicit /app dir header with mode 0700 and uid/gid 1000. + var buf0 bytes.Buffer + tw0 := tar.NewWriter(&buf0) + require.NoError(t, tw0.WriteHeader(&tar.Header{ + Typeflag: tar.TypeDir, + Name: "app/", + Mode: 0700, + Uid: 1000, + Gid: 1000, + })) + require.NoError(t, tw0.Close()) + tree0, err := ParseLayerTar(&buf0) + require.NoError(t, err) + + // Layer 1: only app/new.txt — no explicit app/ header, so insertNode + // creates an inferred placeholder for app/ with mode 0755 and uid/gid 0. + var buf1 bytes.Buffer + tw1 := tar.NewWriter(&buf1) + require.NoError(t, tw1.WriteHeader(&tar.Header{ + Typeflag: tar.TypeReg, + Name: "app/new.txt", + Size: 10, + Mode: 0644, + })) + _, err = tw1.Write(make([]byte, 10)) + require.NoError(t, err) + require.NoError(t, tw1.Close()) + tree1, err := ParseLayerTar(&buf1) + require.NoError(t, err) + + layers := []Layer{ + {Index: 0, Tree: tree0}, + {Index: 1, Tree: tree1}, + } + stacked := Stack(layers) + require.Len(t, stacked, 2) + + app := stacked[1].Root.FindChild("app") + require.NotNil(t, app) + assert.Equal(t, fs.ModeDir|fs.FileMode(0700), app.Mode, + "layer 0's explicit mode 0700 must survive the implicit parent in layer 1") + assert.Equal(t, 1000, app.UID, "UID must not be reset to 0 by the inferred placeholder") + assert.Equal(t, 1000, app.GID, "GID must not be reset to 0 by the inferred placeholder") +} diff --git a/image/tree_parser.go b/image/tree_parser.go index 7883489..e89c89a 100644 --- a/image/tree_parser.go +++ b/image/tree_parser.go @@ -49,7 +49,7 @@ func ParseLayerTar(r io.Reader) (*FileTree, error) { uid := hdr.Uid gid := hdr.Gid - insertNode(tree.Root, name, size, isDir, isHardlink, hdr.Linkname, mode, uid, gid) + insertNode(tree.Root, name, size, isDir, isHardlink, cleanTarPath(hdr.Linkname), mode, uid, gid) } return tree, nil @@ -103,6 +103,7 @@ func insertNode(root *FileNode, fullPath string, size int64, isDir, isHardlink b existing.GID = gid existing.IsHardlink = isHardlink existing.Linkname = linkname + existing.Inferred = false } else { node := &FileNode{ Name: part, @@ -121,10 +122,11 @@ func insertNode(root *FileNode, fullPath string, size int64, isDir, isHardlink b if existing == nil { dirPath := "/" + path.Join(parts[:i+1]...) existing = &FileNode{ - Name: part, - Path: dirPath, - IsDir: true, - Mode: fs.ModeDir | 0755, + Name: part, + Path: dirPath, + IsDir: true, + Mode: fs.ModeDir | 0755, + Inferred: true, } current.AddChild(existing) } else if !existing.IsDir { @@ -137,6 +139,7 @@ func insertNode(root *FileNode, fullPath string, size int64, isDir, isHardlink b existing.Linkname = "" existing.IsHardlink = false existing.Mode = fs.ModeDir | 0755 + existing.Inferred = true } current = existing }