Skip to content

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

Draft
crazy-max wants to merge 2 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 2 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, 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.

{0x42, 0x5A, 0x68}, // bzip2
{0x1F, 0x8B, 0x08}, // gzip
{0xFD, 0x37, 0x7A, 0x58, 0x5A, 0x00}, // xz
{0x28, 0xB5, 0x2F, 0xFD}, // zstd

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.

I added zstd to this archive sniffing list because the previous go-archive decompression path already supported zstd archives. Without this magic value, the new decompression helper can handle zstd, but HTTP context archive detection would still fail to classify zstd-compressed tar streams as archives.

@crazy-max
crazy-max force-pushed the replace-go-archive-with-containerd branch 2 times, most recently from a91199d to f77f604 Compare August 12, 2026 09:58
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>
@crazy-max
crazy-max force-pushed the replace-go-archive-with-containerd branch 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 2 times, most recently from 8f8a223 to f58b77c Compare August 12, 2026 13:08
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 f58b77c to 0bad7f4 Compare August 12, 2026 13:55
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