Materialize per-layer artifacts with explicit whiteout handling

[chruffins]

summary

Stacked stage of the image-storage project. Adds a per-layer artifact store at images/layers/<layer-blob-digest>/:

• Materialization (materializeLayerArtifact): reads the compressed layer blob from the existing shared OCI cache (no new downloader), unpacks it into an isolated temp directory, converts to erofs, and installs layer.erofs atomically beside an artifact.json record. Keyed by layer blob digest plus format/options, so identical layers shared across image versions materialize once.
• Whiteout model: .wh.<name> and .wh..wh..opq markers are recorded in the artifact record (dir, target, opaque) instead of being assumed to compose.
• Composition primitive (applyLayerTree): merges one unpacked layer into a target tree with correct OCI semantics — whiteouts/opaque markers remove or mask lower-layer content first, then the layer's own entries are copied on top, with whiteout-then-recreate pairs resolved correctly. Raw tar whiteout files are interpreted explicitly; they are never passed to overlayfs. Symlinks are removed rather than followed during deletion; hardlinks within a layer stay linked; path traversal is confined.
• Interrupted builds leave only temp files; the next materialization rebuilds cleanly. Completed artifacts are reused without rework.

validation

Synthetic OCI layouts and hand-built trees cover: materialization fields and reuse, missing-blob errors, whiteout/opaque inventory, whiteout/opaque/type-replacement/same-layer-recreate semantics, and symlink/hardlink edge cases. go test ./lib/images green including -race; Docker Hub-backed tests are intermittently rate-limited in this environment.

---

Note

Medium Risk
New tar unpack and layer-merge logic is security- and correctness-sensitive (path escapes, whiteout semantics, device nodes); it reuses the existing EROFS toolchain but is foundational for future image assembly.

Overview
Introduces a content-addressed per-layer store under images/layers/<layer-blob-digest>/, with path helpers for layer.erofs and artifact.json.

materializeLayerArtifact builds or reuses a layer artifact from the existing OCI cache blob: unpack (gzip/zstd tar) into a temp dir, record metadata including whiteout inventory (.wh.* and opaque .wh..wh..opq), convert to EROFS via existing convertToErofs, and install atomically. Missing blobs fail clearly; completed artifacts are not rebuilt.

applyLayerTree is the composition primitive that merges one unpacked layer onto a target tree with explicit OCI semantics—apply whiteouts/opaque dirs against lower content first, then copy layer entries (hardlinks, symlinks, type replacements) without leaking whiteout marker files. Tar extraction uses path confinement and full entry types where supported.

Tests cover materialization, reuse, whiteout recording, composition edge cases, and symlink/hardlink behavior.

^{Reviewed by Cursor Bugbot for commit 2ea330e. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 5 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 2ea330e. Configure here.}

[cursor]

Tar extraction follows intermediate symlinks

High Severity

safeJoin only lexically cleans tar names, and extractTarEntry then creates files with os.OpenFile and parents with os.MkdirAll, both of which follow host symlinks. A layer that plants a symlink and later writes through it can escape the unpack directory onto the host. The repo already uses filepath-securejoin and O_NOFOLLOW for this in lib/volumes/archive.go.

Additional Locations (1)

• lib/images/layer_artifact.go#L286-L292

 

^{Reviewed by Cursor Bugbot for commit 2ea330e. Configure here.}

[cursor]

Directory whiteouts fail silently

High Severity

removePath uses os.Remove, which cannot delete a non-empty directory and ignores the error. OCI .wh.<name> markers hide files or directories, so a whiteout of a populated directory leaves lower-layer contents in place. The same helper also blocks replacing a directory with a symlink during copy.

Additional Locations (2)

• lib/images/layer_artifact.go#L359-L364
• lib/images/layer_artifact.go#L465-L468

 

^{Reviewed by Cursor Bugbot for commit 2ea330e. Configure here.}

[cursor]

Opaque whiteouts follow target symlinks

High Severity

clearDirContents is documented not to follow symlinks, but os.ReadDir does. If a lower layer left a symlink at the opaque directory path, phase 1 lists and deletes the symlink target’s contents instead of removing the link. That can destroy files outside the composed tree before phase 2 replaces the path.

Additional Locations (1)

• lib/images/layer_artifact.go#L356-L358

 

^{Reviewed by Cursor Bugbot for commit 2ea330e. Configure here.}

[cursor]

Device copy uses wrong file mode

Medium Severity

copyEntryInto recreates devices and fifos with unix.Mknod using uint32(info.Mode()). Go fs.FileMode type bits are not Unix S_IFCHR/S_IFBLK/S_IFIFO, so mknod gets a mode with no file type and fails with EINVAL. Composition then aborts on layers that contain device nodes or fifos.

 

^{Reviewed by Cursor Bugbot for commit 2ea330e. Configure here.}

[cursor]

Layer digest skips path validation

Medium Severity

New layer path helpers join layerHex into images/layers/ without paths.ValidatePathComponent. The call-site check only rejects empty strings and /, so values like .. still escape the layers directory and can write layer.erofs / artifact.json under images/.

Additional Locations (1)

• lib/paths/paths.go#L185-L200

 

^{Triggered by learned rule: Filesystem-backed resource IDs must pass ValidatePathComponent before path construction}

^{Reviewed by Cursor Bugbot for commit 2ea330e. Configure here.}

[github-actions]

Decompression bomb: layer extraction has no cumulative size cap

Semgrep rule: go.lang.security.decompression_bomb.potential-dos-via-decompression-bomb

The rule fired on line 291 (io.Copy(io.Discard, reader)), but that copy targets io.Discard — it burns CPU, not disk. The real instance of the same class is here: every tar.TypeReg entry is written through this unbounded io.Copy(file, tr), and neither extractTarFile nor the loop in unpackLayerBlob enforces any ceiling on total unpacked bytes or entry count.

Why this is a true positive

• Layer blobs and media types come from a remote OCI manifest (lib/images/oci.go:474), i.e. a customer-supplied image reference — the compressed input is attacker-controlled.
• stats.unpackedBytes is accumulated but never compared against a limit, and header.Size is trusted for accounting only.
• The diffID check in materializeLayerArtifact (lib/images/layer_artifact.go:173) runs after extraction finishes, and is skipped entirely when desc.DiffID == "", so it cannot bound the write.
• Disk admission control in lib/resources/resource.go sums images already marked ready; it does not throttle an in-progress unpack. On a shared hypervisor host, a ~1 KB gzip layer expanding to hundreds of GB fills the data partition for every co-tenant VM.

Note this path is currently reached only from layer_artifact_test.go — nothing else calls materializeLayerArtifact yet — so this is latent, and worth fixing before it is wired into the pull path.

Recommended fix — thread an explicit budget through the unpack loop and bound the per-file copy:

const (
    maxUnpackedBytes = 32 << 30 // 32 GiB per layer
    maxEntries       = 1 << 20
)

// unpackLayerBlob: budget := int64(maxUnpackedBytes), and in the loop
if stats.entries++; stats.entries > maxEntries {
    return nil, fmt.Errorf("layer exceeds %d entries", maxEntries)
}

// extractTarFile: refuse to write past the remaining budget
func extractTarFile(tr *tar.Reader, target string, header *tar.Header, budget *int64) error {
    // ...
    n, err := io.CopyN(file, tr, *budget+1)
    if err != nil && err != io.EOF {
        _ = file.Close()
        return err
    }
    if n > *budget {
        _ = file.Close()
        return fmt.Errorf("layer exceeds unpacked size limit of %d bytes", maxUnpackedBytes)
    }
    *budget -= n
    // ...
}

Do not apply Semgrep's suggested autofix at line 291 (io.CopyN(io.Discard, reader, 1024*1024*256)). Truncating that drain leaves the TeeReader hash incomplete, yielding a wrong stats.diffID and spurious "diff id mismatch" failures for any layer over 256 MB. If you want the trailing-data read bounded too, cap it well above the largest expected layer and treat hitting the cap as an error rather than silently stopping.

If you consider this an accepted risk, suppress with either:

• inline on the flagged line:
  // nosemgrep: go.lang.security.decompression_bomb.potential-dos-via-decompression-bomb (or bare // nosemgrep to silence all rules on that line)
• or exclude the file by adding lib/images/layer_artifact.go to .semgrepignore