archive: resolve absolute symlinks and hard links within extraction root - #93
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #93 +/- ##
==========================================
- Coverage 65.81% 65.24% -0.57%
==========================================
Files 42 44 +2
Lines 2039 2325 +286
==========================================
+ Hits 1342 1517 +175
- Misses 519 595 +76
- Partials 178 213 +35 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR updates tar/layer extraction to allow traversing absolute symlinks inside the extraction destination with chroot-like semantics (resolving absolute symlink targets relative to the extraction root), restoring compatibility after the move to os.Root.
Changes:
- Added
resolveArchivePathto resolve intermediate symlinks whenos.Rootcan’t traverse an entry’s parent, while keeping the final component unresolved. - Applied the resolved path handling to both
Untar(inarchive.go) andUnpackLayer(indiff.go), including implied-directory creation and AUFS opaque-whiteout bookkeeping. - Added a regression test for
Untarcovering extraction through an absolute symlink within the destination.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| diff.go | Uses resolveArchivePath during layer extraction and updates AUFS opaque-whiteout path tracking to use resolved native paths. |
| archive.go | Adds resolveArchivePath, routes Untar through it, and updates implied-directory creation to accept resolved root-relative paths. |
| archive_unix_test.go | Adds a regression test validating extraction through an absolute symlink under the destination root. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
98e9c00 to
292abe5
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
diff.go:34
- The comment says
unpackedPathscontains resolved paths, butdstPathis only sometimes resolved (resolveArchivePath is a no-op whenroot.Stat(parent)succeeds). This can be misleading, and it also meansunpackedPathsmay still contain unresolved symlink paths in cases whereos.Rootcan traverse them.
Consider updating the comment to reflect that paths are only resolved when needed (e.g., when os.Root can't traverse the parent), or changing the keying logic to always use a consistently resolved form.
// unpackedPaths tracks resolved, native-separator, root-relative paths
// already written in this layer so that the AUFS opaque-whiteout walk
// knows which paths to preserve.
The move to os.Root caused archive extraction to reject paths that traverse
absolute symlinks inside the destination. For example, given:
var/run -> /run
os.Root interprets /run as an absolute host path and reports that the path
escapes the root. Archive extraction instead requires chroot-like semantics,
where absolute symlink targets are resolved relative to the extraction root.
When os.Root cannot traverse an entry's parent, resolve it with fsRootPath and
continue extraction using the resulting root-relative path. Leave the final
component unresolved because extraction may create or replace it.
Apply this handling to Unpack and UnpackLayer, including implied directories,
whiteouts, deferred directory timestamps, and opaque-whiteout path tracking.
The actual filesystem operations continue to use os.Root and remain confined
to the extraction destination.
This is a compatibility workaround that resolves paths separately from their
use. It should eventually be replaced with handle-relative operations that
provide resolve-in-root semantics.
A regression test was added, which fails before this patch;
=== RUN TestUntarThroughAbsoluteSymlink
=== RUN TestUntarThroughAbsoluteSymlink/existing_target
archive_unix_test.go:558: assertion failed: error is not nil: statat var/run/existing/non-existing: path escapes from parent
=== RUN TestUntarThroughAbsoluteSymlink/missing_target
archive_unix_test.go:558: assertion failed: error is not nil: statat var/run/existing/non-existing: path escapes from parent
--- FAIL: TestUntarThroughAbsoluteSymlink (0.00s)
--- FAIL: TestUntarThroughAbsoluteSymlink/existing_target (0.00s)
--- FAIL: TestUntarThroughAbsoluteSymlink/missing_target (0.00s)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
15f7708 to
6c3be27
Compare
Hardlink extraction failed when the source path crossed an absolute symlink inside a container root because os.Root treated the target as a host-rooted escape. Resolve the validated source with extraction-root semantics before linking, and reuse that bounded path when applying timestamps. Record relative escapes when each symlink is read so a later absolute link cannot make the path eligible for resolve-in-root fallback. Signed-off-by: Paweł Gronowski <git@grono.dev> Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
6c3be27 to
4f6cd58
Compare
…p ci] Bumps the go-modules group in /e2e-go with 2 updates: [github.com/moby/go-archive](https://github.com/moby/go-archive) and [github.com/shirou/gopsutil/v4](https://github.com/shirou/gopsutil). Updates `github.com/moby/go-archive` from 0.2.1 to 0.3.2 Release notes *Sourced from [github.com/moby/go-archive's releases](https://github.com/moby/go-archive/releases).* > v0.3.2 > ------ > > What's Changed > -------------- > > Fix a regression introduced in v0.3.0 that caused archive extraction to fail when paths traversed absolute symlinks inside the destination root, such as `var/run -> /run`. Absolute symlink targets are now resolved relative to the extraction root while relative symlink escapes remain rejected. [moby/go-archive#93](https://redirect.github.com/moby/go-archive/pull/93) > > **Full Changelog**: <moby/go-archive@v0.3.1...v0.3.2> > > v0.3.1 > ------ > > Fixes > ----- > > This patch release fixes a regression introduced in v0.2.1 where archive extraction could fail when an archive omitted explicit entries for parent directories. For example, extracting `etc/dnf/` without a preceding `etc/` entry could return `mkdirat etc/dnf: no such file or directory`. > > This prevented affected images from being extracted. Archive extraction now creates implied parent directories for both file and directory entries. > > What's Changed > -------------- > > * archive: create implied parents for directory entries [moby/go-archive#92](https://redirect.github.com/moby/go-archive/pull/92) > * archive: Tarballer.Go: suppress io.ErrClosedPipe logs on close [moby/go-archive#94](https://redirect.github.com/moby/go-archive/pull/94) > > **Full Changelog**: <moby/go-archive@v0.3.0...v0.3.1> > > v0.3.0 > ------ > > Security > -------- > > This release fixes **CVE-2026-17106** / **[GHSA-hfg8-hc9c-6c3h](https://github.com/moby/go-archive/security/advisories/GHSA-hfg8-hc9c-6c3h)**, where a crafted tar archive could use links to cause extraction operations to create or overwrite files outside the intended destination directory. > > The issue affected `Unpack`, `UnpackLayer`, `Untar`, `UntarUncompressed`, and the `ApplyLayer` helpers. Users should upgrade and avoid extracting untrusted archives with earlier versions. > > What's Changed > -------------- > > * archive: harden tar extraction against path traversal [moby/go-archive#45](https://redirect.github.com/moby/go-archive/pull/45) > * archive: do not follow reparse points in chtimes [moby/go-archive#90](https://redirect.github.com/moby/go-archive/pull/90) > * archive: fix creation time updates on Windows [moby/go-archive#79](https://redirect.github.com/moby/go-archive/pull/79) > * archive: minor cleanups and godoc touch-up [moby/go-archive#87](https://redirect.github.com/moby/go-archive/pull/87) > * archive: RebaseArchiveEntries: fix archive path rebasing [moby/go-archive#43](https://redirect.github.com/moby/go-archive/pull/43) > > Test and CI changes > ------------------- > > * ci: enable dependabot for actions [moby/go-archive#81](https://redirect.github.com/moby/go-archive/pull/81) > * archive: make breakoutErr unwrap its cause [moby/go-archive#91](https://redirect.github.com/moby/go-archive/pull/91) > * archive: use filepath for filesystem paths in tests [moby/go-archive#80](https://redirect.github.com/moby/go-archive/pull/80) > * archive: use filepath for filesystem paths in tests [moby/go-archive#80](https://redirect.github.com/moby/go-archive/pull/80) > > **Full Changelog**: <moby/go-archive@v0.2.1...v0.3.0> Commits * [`9e6d2c7`](moby/go-archive@9e6d2c7) Merge pull request [#93](https://redirect.github.com/moby/go-archive/issues/93) from thaJeztah/fix\_absolute\_symlinks * [`4f6cd58`](moby/go-archive@4f6cd58) archive: resolve hardlinks through absolute symlinks * [`e564ecc`](moby/go-archive@e564ecc) archive: resolve absolute symlinks within extraction root * [`5bb8a45`](moby/go-archive@5bb8a45) Merge pull request [#94](https://redirect.github.com/moby/go-archive/issues/94) from thaJeztah/denoise * [`1bec7ec`](moby/go-archive@1bec7ec) archive: Tarballer.Go: suppress io.ErrClosedPipe logs on close * [`279fa6d`](moby/go-archive@279fa6d) Merge pull request [#92](https://redirect.github.com/moby/go-archive/issues/92) from thaJeztah/fix\_implied\_directories * [`517985a`](moby/go-archive@517985a) archive: create implied parents for directory entries * [`1c23372`](moby/go-archive@1c23372) Merge pull request [#43](https://redirect.github.com/moby/go-archive/issues/43) from thaJeztah/fix\_rebase\_from\_root * [`8829a25`](moby/go-archive@8829a25) RebaseArchiveEntries: fix archive path rebasing * [`c583b20`](moby/go-archive@c583b20) Merge pull request [#90](https://redirect.github.com/moby/go-archive/issues/90) from thaJeztah/chtimes\_nofollow * Additional commits viewable in [compare view](moby/go-archive@v0.2.1...v0.3.2) Updates `github.com/shirou/gopsutil/v4` from 4.26.6 to 4.26.7 Release notes *Sourced from [github.com/shirou/gopsutil/v4's releases](https://github.com/shirou/gopsutil/releases).* > v4.26.7 > ------- > > What's Changed > -------------- > > ### cpu > > * fix: harden parsers against malformed/truncated input by [`@shirou`](https://github.com/shirou) in [shirou/gopsutil#2109](https://redirect.github.com/shirou/gopsutil/pull/2109) > * [cpu][windows]: compute cpu-total times from integer ticks by [`@skartikey`](https://github.com/skartikey) in [shirou/gopsutil#2111](https://redirect.github.com/shirou/gopsutil/pull/2111) > * [darwin][process]: fix errno handling and library lifetime on darwin by [`@shirou`](https://github.com/shirou) in [shirou/gopsutil#2119](https://redirect.github.com/shirou/gopsutil/pull/2119) > * [cpu][windows]: compute total counters from individual stats to handle processor groups correctly by [`@srebhan`](https://github.com/srebhan) in [shirou/gopsutil#2125](https://redirect.github.com/shirou/gopsutil/pull/2125) > * [cpu][windows]: harden the cpu-total computation added in [#2125](https://redirect.github.com/shirou/gopsutil/issues/2125) by [`@shirou`](https://github.com/shirou) in [shirou/gopsutil#2128](https://redirect.github.com/shirou/gopsutil/pull/2128) > > ### net > > * fix(net): pad GetExtendedTcpTable buffer to prevent GC thrashing on Windows by [`@HarshalPatel1972`](https://github.com/HarshalPatel1972) in [shirou/gopsutil#2108](https://redirect.github.com/shirou/gopsutil/pull/2108) > > ### process > > * process: implement Darwin IOCounters via proc\_pid\_rusage by [`@DavRack`](https://github.com/DavRack) in [shirou/gopsutil#2117](https://redirect.github.com/shirou/gopsutil/pull/2117) > > ### other > > * feat: add psutil comparison tests for cpu, mem and load by [`@shirou`](https://github.com/shirou) in [shirou/gopsutil#2114](https://redirect.github.com/shirou/gopsutil/pull/2114) > > New Contributors > ---------------- > > * [`@DavRack`](https://github.com/DavRack) made their first contribution in [shirou/gopsutil#2117](https://redirect.github.com/shirou/gopsutil/pull/2117) > * [`@srebhan`](https://github.com/srebhan) made their first contribution in [shirou/gopsutil#2125](https://redirect.github.com/shirou/gopsutil/pull/2125) > > **Full Changelog**: <shirou/gopsutil@v4.26.6...v4.26.7> Commits * [`52a24c8`](shirou/gopsutil@52a24c8) Merge pull request [#2128](https://redirect.github.com/shirou/gopsutil/issues/2128) from shirou/feat/follow-up-2125 * [`268a953`](shirou/gopsutil@268a953) [cpu][windows]: harden the cpu-total computation added in [#2125](https://redirect.github.com/shirou/gopsutil/issues/2125) * [`1e34da6`](shirou/gopsutil@1e34da6) Merge pull request [#2125](https://redirect.github.com/shirou/gopsutil/issues/2125) from srebhan/fix\_cpu\_windows\_total * [`61f8802`](shirou/gopsutil@61f8802) Merge pull request [#2122](https://redirect.github.com/shirou/gopsutil/issues/2122) from shirou/dependabot/github\_actions/actions/checko... * [`7fb4dcf`](shirou/gopsutil@7fb4dcf) Merge pull request [#2123](https://redirect.github.com/shirou/gopsutil/issues/2123) from shirou/dependabot/github\_actions/actions/setup-... * [`ae7d91a`](shirou/gopsutil@ae7d91a) Merge pull request [#2119](https://redirect.github.com/shirou/gopsutil/issues/2119) from shirou/fix/darwin-errno-and-libcache * [`49052a1`](shirou/gopsutil@49052a1) [darwin][process]: use a PID above PID\_MAX in the not-running tests * [`991b238`](shirou/gopsutil@991b238) [darwin]: pass the remaining Go pointers as unsafe.Pointer on darwin * [`b9930e2`](shirou/gopsutil@b9930e2) Merge pull request [#2124](https://redirect.github.com/shirou/gopsutil/issues/2124) from shirou/dependabot/github\_actions/actions/labele... * [`38a01b4`](shirou/gopsutil@38a01b4) [cpu][windows]: compute total counters from individual stats to handle proces... * Additional commits viewable in [compare view](shirou/gopsutil@v4.26.6...v4.26.7) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- Dependabot commands and options You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore major version` will close this group update PR and stop Dependabot creating any more for the specific dependency's major version (unless you unignore this specific dependency's major version or upgrade to it yourself) - `@dependabot ignore minor version` will close this group update PR and stop Dependabot creating any more for the specific dependency's minor version (unless you unignore this specific dependency's minor version or upgrade to it yourself) - `@dependabot ignore ` will close this group update PR and stop Dependabot creating any more for the specific dependency (unless you unignore this specific dependency or upgrade to it yourself) - `@dependabot unignore ` will remove all of the ignore conditions of the specified dependency - `@dependabot unignore ` will remove the ignore condition of the specified dependency and ignore conditions
CopyToContainerregression in 29.7.0: "path escapes from parent" when copying into paths that traverse/var/run → /runsymlink moby#53258The move to os.Root caused archive extraction to reject paths that traverse absolute symlinks inside the destination. For example, given:
os.Root interprets /run as an absolute host path and reports that the path escapes the root. Archive extraction instead requires chroot-like semantics, where absolute symlink targets are resolved relative to the extraction root.
When os.Root cannot traverse an entry's parent, resolve it with fsRootPath and continue extraction using the resulting root-relative path. Leave the final component unresolved because extraction may create or replace it.
Apply this handling to Unpack and UnpackLayer, including implied directories, whiteouts, deferred directory timestamps, and opaque-whiteout path tracking. The actual filesystem operations continue to use os.Root and remain confined to the extraction destination.
This is a compatibility workaround that resolves paths separately from their use. It should eventually be replaced with handle-relative operations that provide resolve-in-root semantics.
A regression test was added, which fails before this patch;
Release note: