archive: create implied parents for directory entries - #92
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #92 +/- ##
==========================================
- Coverage 65.81% 65.15% -0.67%
==========================================
Files 42 44 +2
Lines 2039 2270 +231
==========================================
+ Hits 1342 1479 +137
- Misses 519 592 +73
- Partials 178 199 +21 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1682608 to
178d2a3
Compare
Tar archives are not required to contain headers for every parent
directory. A directory entry can therefore itself have implied parents.
For example, an archive may contain:
etc/dnf/
etc/dnf/dnf.conf
without containing an entry for `etc/`.
Create implied parent directories regardless of the type of the final
entry. The existing root-parent check still prevents attempting to
create a parent for top-level entries.
This restores the effective behavior from before go-archive v0.2.1.
The original implementation normalized `hdr.Name` with
`filepath.Clean`, then used a trailing path separator as an apparent
root check.[1] Its comments described this as an "is-root check" and
"Not the root directory".
Because `filepath.Clean` removes trailing separators from non-root
paths, a directory entry such as `etc/dnf/` became `etc/dnf` before
reaching the check. Its implied parent, `etc`, was therefore created.
The filesystem root `/` retained its trailing separator and was skipped;
`.` and `./` entered the block but resulted in a harmless check of the
already existing extraction root.
The code was later moved into the `createImpliedDirectories` helper.[2]
That extraction separated the trailing-separator check from the earlier
`filepath.Clean` call, obscuring that directory entries had already lost
their trailing separator before reaching the check.
Commit e45dc89 then replaced the ineffective trailing-separator check
with `hdr.Typeflag != tar.TypeDir`.[3] Although this appeared to correct
the directory detection, it changed the effective behavior: implied
parents were no longer created when the final entry was itself a
directory.
Both files and directories may have parents implied by their paths.
Remove the entry-type check and rely on the existing `parent == "." ||
parent == ""` guard to skip the extraction root.
[1]: moby/moby@a4868e2
[2]: moby@29b0f33
[3]: moby@e45dc89
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
178d2a3 to
517985a
Compare
| }, { | ||
| // Deliberately omit the trailing slash to match the normalized path passed | ||
| // to createImpliedDirectories; Typeflag is the authoritative directory marker. | ||
| // | ||
| // Regression test for https://github.com/moby/moby/issues/53257 | ||
| Name: "implied/dir-without-trailing-slash", | ||
| Typeflag: tar.TypeDir, | ||
| Mode: 0o700, |
There was a problem hiding this comment.
Failing before this patch;
=== RUN TestImpliedDirectoryPermissions/umask=022
archive_unix_test.go:113: assertion failed: error is not nil: mkdirat implied/dir-without-trailing-slash: no such file or directory
=== RUN TestImpliedDirectoryPermissions/umask=027
archive_unix_test.go:113: assertion failed: error is not nil: mkdirat implied/dir-without-trailing-slash: no such file or directory
--- FAIL: TestImpliedDirectoryPermissions (0.00s)
--- FAIL: TestImpliedDirectoryPermissions/umask=022 (0.00s)
--- FAIL: TestImpliedDirectoryPermissions/umask=027 (0.00s)
|
|
There was a problem hiding this comment.
Pull request overview
This PR restores pre-v0.2.1 behavior during tar extraction by ensuring implied parent directories are created for all entries, including directory entries (not just non-directories). This aligns with tar’s allowance for archives to omit explicit headers for parent directories (e.g., having etc/dnf/ without etc/) and addresses the regression described in moby/moby#53257.
Changes:
- Remove the
hdr.Typeflag != tar.TypeDirguard so implied parents are created even when the current entry is itself a directory. - Keep the existing “root parent” skip (
parent == "." || parent == "") to avoid attempting to create parents for top-level entries. - Add a regression case to
TestImpliedDirectoryPermissionsthat would fail extraction if implied parents aren’t created for a directory entry.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| archive.go | Ensures implied parent directories are created regardless of entry type (directory vs non-directory). |
| archive_unix_test.go | Adds a regression entry to validate directory entries without explicit parent headers extract successfully. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Yeah that diff without whitespace disabled is very noisy! |
|
It is! I originally was considering an early return, but didn't do so .. exactly for this reason 😂 |
…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
This PR contains the following updates: | Package | Type | Update | Change | Pending | [Age](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) | |---|---|---|---|---|---|---| | docker | stage | minor | `29.6.2-dind-rootless` → `29.7.1-dind-rootless` | |  |  | | docker | stage | minor | `29.6.2-dind` → `29.7.1-dind` | |  |  | | [github.com/docker/cli](https://github.com/docker/cli) | require | minor | `v29.6.2+incompatible` → `v29.7.1+incompatible` | `v29.7.2+incompatible` |  |  | | [github.com/moby/go-archive](https://github.com/moby/go-archive) | require | minor | `v0.2.1` → `v0.3.2` | `v0.3.3` |  |  | --- ### Release Notes <details> <summary>docker/cli (github.com/docker/cli)</summary> ### [`v29.7.1+incompatible`](docker/cli@v29.7.0...v29.7.1) [Compare Source](docker/cli@v29.7.0...v29.7.1) ### [`v29.7.0+incompatible`](docker/cli@v29.6.2...v29.7.0) [Compare Source](docker/cli@v29.6.2...v29.7.0) </details> <details> <summary>moby/go-archive (github.com/moby/go-archive)</summary> ### [`v0.3.2`](https://github.com/moby/go-archive/releases/tag/v0.3.2) [Compare Source](moby/go-archive@v0.3.1...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. [#​93](moby/go-archive#93) **Full Changelog**: <moby/go-archive@v0.3.1...v0.3.2> ### [`v0.3.1`](https://github.com/moby/go-archive/releases/tag/v0.3.1) [Compare Source](moby/go-archive@v0.3.0...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 [#​92](moby/go-archive#92) - archive: Tarballer.Go: suppress io.ErrClosedPipe logs on close [#​94](moby/go-archive#94) **Full Changelog**: <moby/go-archive@v0.3.0...v0.3.1> ### [`v0.3.0`](https://github.com/moby/go-archive/releases/tag/v0.3.0) [Compare Source](moby/go-archive@v0.2.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 [#​45](moby/go-archive#45) - archive: do not follow reparse points in chtimes [#​90](moby/go-archive#90) - archive: fix creation time updates on Windows [#​79](moby/go-archive#79) - archive: minor cleanups and godoc touch-up [#​87](moby/go-archive#87) - archive: RebaseArchiveEntries: fix archive path rebasing [#​43](moby/go-archive#43) #### Test and CI changes - ci: enable dependabot for actions [#​81](moby/go-archive#81) - archive: make breakoutErr unwrap its cause [#​91](moby/go-archive#91) - archive: use filepath for filesystem paths in tests [#​80](moby/go-archive#80) - archive: use filepath for filesystem paths in tests [#​80](moby/go-archive#80) **Full Changelog**: <moby/go-archive@v0.2.1...v0.3.0> </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - Between 12:00 AM and 03:59 AM, only on Monday (`* 0-3 * * 1`) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xOTEuMiIsInVwZGF0ZWRJblZlciI6IjQzLjE5MS4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119--> Reviewed-on: https://gitea.com/gitea/runner/pulls/1160 Reviewed-by: silverwind <2021+silverwind@noreply.gitea.com> Co-authored-by: Renovate Bot <renovate-bot@gitea.com>
relates to:
Tar archives are not required to contain headers for every parent directory. A directory entry can therefore itself have implied parents. For example, an archive may contain:
without containing an entry for
etc/.Create implied parent directories regardless of the type of the final entry. The existing root-parent check still prevents attempting to create a parent for top-level entries.
This restores the effective behavior from before go-archive v0.2.1.
The original implementation normalized
hdr.Namewithfilepath.Clean, then used a trailing path separator as an apparent root check.1 Its comments described this as an "is-root check" and "Not the root directory".Because
filepath.Cleanremoves trailing separators from non-root paths, a directory entry such asetc/dnf/becameetc/dnfbefore reaching the check. Its implied parent,etc, was therefore created. The filesystem root/retained its trailing separator and was skipped;.and./entered the block but resulted in a harmless check of the already existing extraction root.The code was later moved into the
createImpliedDirectorieshelper.2 That extraction separated the trailing-separator check from the earlierfilepath.Cleancall, obscuring that directory entries had already lost their trailing separator before reaching the check.Commit e45dc89 then replaced the ineffective trailing-separator check with
hdr.Typeflag != tar.TypeDir.3 Although this appeared to correct the directory detection, it changed the effective behavior: implied parents were no longer created when the final entry was itself a directory.Both files and directories may have parents implied by their paths. Remove the entry-type check and rely on the existing
parent == "." || parent == ""guard to skip the extraction root.