Skip to content

remove go-archive and bound ADD archive extraction - #7034

Draft
crazy-max wants to merge 4 commits into
moby:masterfrom
crazy-max:replace-go-archive-with-containerd
Draft

remove go-archive and bound ADD archive extraction#7034
crazy-max wants to merge 4 commits into
moby:masterfrom
crazy-max:replace-go-archive-with-containerd

Conversation

@crazy-max

@crazy-max crazy-max commented Aug 12, 2026

Copy link
Copy Markdown
Member

closes #6965
closes #6964

This removes github.com/moby/go-archive from Dockerfile ADD archive handling instead of updating the vendored module. Archive probing now uses a small local decompression helper for Docker-compatible gzip, bzip2, xz, zstd, and uncompressed tar streams, while extraction preserves the existing ownership mapping, whiteout-file behavior, best-effort SCHILY.xattr.* restoration, and Windows no-owner behavior.

The extraction path is intentionally not just a direct replacement with containerd archive.Apply, because that doesn't recreate the old chrootarchive.Untar boundary. ADD archives are now extracted through os.Root so archive paths, hardlinks, archive-created symlinks, and preexisting symlinks cannot write outside the destination root, which keeps the replacement aligned with the escape class described in GHSA-hfg8-hc9c-6c3h.

Special tar entries for block devices, character devices, and FIFOs are skipped before touching the destination path, so archives containing those entries don't fail the whole ADD and don't replace existing files.

Comment thread frontend/dockerui/context.go Outdated
@crazy-max
crazy-max force-pushed the replace-go-archive-with-containerd branch 3 times, most recently from f77f604 to 467d8c1 Compare August 12, 2026 10:07
Comment thread solver/llbsolver/file/unpack.go Outdated
GID: u.GID,
opts = append(opts, unpackPlatformApplyOpts()...)

_, err = archive.Apply(ctx, dest, rdr, opts...)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum it doesn't perform extraction inside a chroot 🙈. It relies on archive path validation and guarded filesystem operations instead. Given that this code handles user-provided Dockerfile ADD archives, an explicit extraction-root boundary here would be needed so symlinks, hardlinks, absolute paths, or .. paths cannot write outside the destination.

@crazy-max crazy-max Aug 12, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I guess best is to still rely on go-archive or extract through os.Root ourselves.

@crazy-max crazy-max changed the title replace go-archive with containerd archive apply remove go-archive and bound ADD archive extraction Aug 12, 2026
@crazy-max
crazy-max force-pushed the replace-go-archive-with-containerd branch 3 times, most recently from f58b77c to 0bad7f4 Compare August 12, 2026 13:55
@crazy-max
crazy-max force-pushed the replace-go-archive-with-containerd branch 2 times, most recently from 7e2b951 to 1e7db68 Compare September 1, 2026 14:01
This removes github.com/moby/go-archive from Dockerfile ADD extraction and
archive probing. Extraction now uses containerd's archive.Apply, with a
small local decompression helper for Docker-compatible formats while preserving
whiteout and ownership mapping behavior.

Tests cover symlink and hardlink escape attempts around the extraction root so this
does not regress the confinement expected from the old chrootarchive path.

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
Replace direct containerd archive application in the Dockerfile ADD unpack path
with a local tar extractor that opens the destination through os.Root. This
keeps extraction operations scoped to the destination root while preserving regular
files, directories, symlinks, hardlinks, whiteout files, ownership mapping, and
Windows no-owner behavior.

Add coverage for parent-directory paths, Windows volume paths, hardlinks outside
the destination, and symlink traversal through both archive-created and preexisting
symlinks.

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
@crazy-max
crazy-max force-pushed the replace-go-archive-with-containerd branch from 1e7db68 to 6ebd396 Compare September 2, 2026 08:03
Normalize tar paths with POSIX semantics before converting them to host paths so traversal entries
and Windows-only path forms are rejected while absolute archive names and hardlink targets remain
root-relative.

Apply Unix modes without following the final path component, clamp unsupported archive timestamps,
preserve symlink timestamps where supported, and cover traversal, hardlink, symlink, special-file,
timestamp, and umask behavior with focused tests.

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
@crazy-max
crazy-max force-pushed the replace-go-archive-with-containerd branch from 6ebd396 to 0c3b969 Compare September 2, 2026 08:23
Restore SCHILY.xattr.* PAX records when unpacking ADD archives, using os.Root-scoped file descriptors
and tolerating unsupported or permission-denied xattr failures like the previous go-archive
BestEffortXattrs path.

Skip block, char, and fifo tar entries before touching the destination so archives containing special
files do not fail the whole ADD or replace existing paths.

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
@crazy-max
crazy-max requested a review from tonistiigi September 2, 2026 08:48
@crazy-max crazy-max added this to the v0.34.0 milestone Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant