vendor: github.com/moby/go-archive main / v0.3.0-dev

[thaJeztah]

• relates to archive: harden tar extraction against path traversal moby/go-archive#45
• relates to (not yet included); archive: cache parent directory fd during tar extraction moby/go-archive#49

vendor: github.com/moby/go-archive main / v0.3.0-dev

full diff: moby/go-archive@v0.2.0...91b7e54

vendor: github.com/moby/go-archive with hardening

full diff: ctalledo/go-archive@91b7e54...df2ce62

- Human readable description for the release notes

- A picture of a cute animal (not mandatory but encouraged)

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[docker-agent]

Assessment: 🟡 NEEDS ATTENTION

This vendor update of github.com/moby/go-archive introduces significant security hardening — os.Root-based filesystem operations with openat(2) semantics, filepath.IsLocal path validation, and improved AUFS whiteout handling. The overall direction is a clear improvement.

Two security findings in the new code warrant attention before merging:

1. TOCTOU in opaque-whiteout walk (diff.go:151): os.RemoveAll(p) uses an absolute path outside the os.Root containment boundary, unlike every other removal in the same function.
2. Empty tar entry name (diff.go:62): A tar entry with an empty name normalizes to "." (the extraction root), passes filepath.IsLocal, and can overwrite root directory metadata.

[docker-agent]

Assessment: 🟢 APPROVE

This vendor update introduces -based containment for tar extraction (using semantics), replaces manual path-traversal checks with , hardens device-number overflow detection, and updates the implementation to use a parent-directory file descriptor. The security model is sound and the hardening changes are a clear improvement over the previous string-prefix checks.

[docker-agent]

Assessment: 🟢 APPROVE

This PR vendors github.com/moby/go-archive from v0.2.0 to a pre-release dev version (v0.2.1-0.20260716170833 via a replace directive pointing to ctalledo's fork). The core changes are security hardening of the tar extraction path: Unpack and createTarFile now use Go's os.Root API (openat-backed bounded filesystem operations) instead of raw os.* calls with manual prefix checks, and path traversal validation is strengthened with filepath.IsLocal after path.Clean.

The hardening changes are well-structured. The os.Root API provides the primary containment guarantee for all path operations. All drafter findings were low-severity and, on deeper analysis, confirmed to be either correct behavior or no-bug outcomes. No high or medium severity issues were identified.

Note: vendor.mod contains a // FIXME comment indicating this uses a personal fork (ctalledo/go-archive) for testing moby/go-archive#45. This is explicitly called out in the PR description and is expected for a dev-version vendor bump.

[docker-agent]

Assessment: 🔴 CRITICAL

This PR vendors an updated github.com/moby/go-archive (v0.3.0-dev) with security hardening, but the vendor.mod includes a replace directive that substitutes the canonical module with a personal testing fork (ctalledo/go-archive). This was intentionally marked FIXME and must be removed before merging.

Lower-confidence findings (not posted inline)

None

[docker-agent]

[high] replace directive points to a personal testing fork instead of the canonical moby/go-archive

The PR adds a replace directive in vendor.mod that redirects github.com/moby/go-archive to github.com/ctalledo/go-archive v0.2.1-0.20260717121834-2bc39549db4e. The adjacent FIXME(thaJeztah) comment explicitly marks this as a temporary testing state while waiting for moby/go-archive#45 to land.

Impact if merged: Every build of docker/cli at this commit uses code from the unreviewed personal fork (ctalledo/go-archive) rather than the canonical moby/go-archive — a supply-chain risk that defeats the stated goal of the PR. The require line correctly points to the official pre-release (v0.2.1-0.20260716170833-91b7e546daf2), but the replace directive overrides it at build time.

Resolution: Remove the replace directive (and the accompanying FIXME comment) before merging, once PR #45 in moby/go-archive is merged and the canonical module is published.

Confidence	Score
🟢 strong	100/100

[thaJeztah]

This is temporary; upstream changes have not been merged yet

[docker-agent]

Understood — I agree this should remain until moby/go-archive#45 is merged upstream. My finding was to flag this as blocking merge, not to suggest removing it from the branch during testing.

Once the upstream PR lands and the canonical module is published, this replace directive should be removed before merging this PR into the main branch. Until then, the temporary fork is the correct approach for validation.

[docker-agent]

Assessment: 🟢 APPROVE

This PR vendors github.com/moby/go-archive at a dev pre-release (v0.3.0-dev / main) and applies a security-hardening layer from ctalledo/go-archive. The changes are well-structured and represent a clear improvement over the previous extraction logic.

Hardening changes reviewed:

• Unpack/UnpackLayer now use os.Root (openat(2)-based) for all extraction operations, preventing symlink-escape attacks at the OS level.
• Path validation replaces the old filepath.Clean + filepath.Rel breakout check with filepath.IsLocal + path.Clean, which is more robust.
• chmodNoSymlink uses fchmodat(2) with AT_SYMLINK_NOFOLLOW (with documented fallback for older kernels that matches or improves on previous os.Chmod behavior).
• lchtimes is now fd-based (unix.UtimesNanoAt via parent directory FD), correctly scoped within the extraction root.
• fsRootPath/walkLinks provide correct symlink bounding for operations os.Root cannot handle (mknod, xattrs) — absolute symlink targets are correctly re-bounded to root on the subsequent iteration.
• The unpackedPaths key change (absolute → POSIX-relative) is consistent: path.Clean removes trailing slashes before storage, and the opaque-whiteout lookup uses matching normalization.
• Error-handling cleanups (_ = writer.Close(), combined defers) are all correct.

No bugs were found in the introduced + lines.

[docker-agent]

Assessment: 🟢 APPROVE

This vendor bump for github.com/moby/go-archive introduces os.Root-based path containment hardening across the archive extraction pipeline. The changes are well-structured and the security model is sound. One minor finding in the fallback chmod path.

[docker-agent]

[low] chmodNoSymlink fallback opens O_RDONLY on a file that may have no read permission

The fallback path (triggered on kernels older than 6.6 that return EOPNOTSUPP for fchmodat with AT_SYMLINK_NOFOLLOW) opens the just-extracted file with O_RDONLY|O_NOFOLLOW|O_NONBLOCK via openat(2). If the file was extracted with a mode that grants no read permission (e.g. 0000, 0200, 0020), this openat call fails with EACCES, causing the entire extraction to abort — even though fchmod(2) on an already-open fd is a pure metadata operation that needs no read access.

Trigger: non-root extraction (user namespaces) of a tar entry with mode 0000 or 0200 on a Linux kernel < 6.6.

Impact: chmodNoSymlink returns an error and Unpack/UnpackLayer aborts, preventing valid container images containing such files from being extracted on older kernels.

Suggested fix: use O_PATH|O_NOFOLLOW instead of O_RDONLY|O_NOFOLLOW|O_NONBLOCK. O_PATH doesn't require any file permission, allows fchmod via the fd (through /proc/self/fd/N indirection on older kernels, or directly on Linux ≥ 3.6 with fchmod(fd, mode)), and still prevents symlink following.

Suggested change

	fd, err := unix.Openat(int(parent.Fd()), base, unix.O_RDONLY|unix.O_NOFOLLOW|unix.O_NONBLOCK, 0)
	fd, err := unix.Openat(int(parent.Fd()), base, unix.O_PATH|unix.O_NOFOLLOW, 0)

Confidence	Score
🟡 moderate	62/100

[thaJeztah]

My AI to here's AI;

---

The underlying concern looks valid: O_RDONLY unnecessarily requires read permission, whereas the owner may be allowed to chmod a file without being allowed to read it.
However, the suggested replacement is not valid as written. fchmod(2) on an O_PATH descriptor fails with EBADF; Linux 3.6 added fstat support for O_PATH, not fchmod.
A Linux-specific fallback could potentially open with O_PATH|O_NOFOLLOW and call chmod through /proc/self/fd/<fd>, but that introduces a procfs dependency. We should also add a non-root regression test and confirm the file already has the restrictive mode when this fallback runs before changing the implementation.

[docker-agent]

Thank you for the correction — I misstated the Linux 3.6 fchmod support. You're right that fchmod(2) on an O_PATH fd fails with EBADF; the /proc/self/fd/<fd> indirection is required.

The procfs dependency is a fair concern, though the codebase already relies on /proc in several places (e.g., reexec, getent fallback). If you'd prefer to avoid it here, an alternative is to check whether the file already has the target mode before opening it (via fstatat with AT_SYMLINK_NOFOLLOW), and skip the fchmod entirely if the mode matches — that would sidestep the EACCES issue for the common case where tar entries already have the correct mode on disk.

Re: testing — agreed that a non-root test on a kernel < 6.6 (or with fchmodat2 stubbed to return EOPNOTSUPP) extracting a tar entry with mode 0000 would confirm the current failure and validate any fix.