Skip to content

Fix absolute hard links temp - #96

Closed
thaJeztah wants to merge 3 commits into
moby:mainfrom
thaJeztah:fix_absolute_symlinks_TEMP
Closed

Fix absolute hard links temp#96
thaJeztah wants to merge 3 commits into
moby:mainfrom
thaJeztah:fix_absolute_symlinks_TEMP

Conversation

@thaJeztah

Copy link
Copy Markdown
Member

No description provided.

@thaJeztah thaJeztah changed the title Fix absolute symlinks temp Fix absolute hard links temp Jul 31, 2026
@thaJeztah

Copy link
Copy Markdown
Member Author

Two failures;

=== RUN   TestUntarInvalidSymlink
    archive_test.go:1057: i=4. linkat victim/hello hardlink: no such file or directory
--- FAIL: TestUntarInvalidSymlink (0.00s)
=== RUN   TestApplyLayerInvalidSymlink
    diff_test.go:193: i=4. linkat victim/hello hardlink: no such file or directory
--- FAIL: TestApplyLayerInvalidSymlink (0.00s)

Remaining issue

Resolving hardlink targets through resolveArchivePath exposed that the fallback is currently too broad.

For paths that escape through a relative symlink, such as:

loophole-victim -> ../victim

os.Root correctly returns path escapes from parent. However, resolveArchivePath then falls back to fsRootPath, which rewrites the path back inside the extraction root instead of preserving the breakout error.

The fallback should only apply when the failure is caused by an absolute symlink that needs chroot-like rebasing. Relative symlink escapes must continue to be rejected.

The existing TestUntarInvalidSymlink and TestApplyLayerInvalidSymlink failures confirm this distinction and should not be changed.

@thaJeztah
thaJeztah force-pushed the fix_absolute_symlinks_TEMP branch from f548b62 to db2c101 Compare July 31, 2026 13:53
@codecov-commenter

codecov-commenter commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 71.76471% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.51%. Comparing base (216738e) to head (6c3be27).
⚠️ Report is 67 commits behind head on main.

Files with missing lines Patch % Lines
archive.go 67.39% 9 Missing and 6 partials ⚠️
rootpath.go 75.00% 5 Missing and 2 partials ⚠️
diff.go 75.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #96      +/-   ##
==========================================
- Coverage   65.81%   65.51%   -0.30%     
==========================================
  Files          42       44       +2     
  Lines        2039     2320     +281     
==========================================
+ Hits         1342     1520     +178     
- Misses        519      588      +69     
- Partials      178      212      +34     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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>
@thaJeztah
thaJeztah force-pushed the fix_absolute_symlinks_TEMP branch from 895d6fe to f01aca2 Compare July 31, 2026 14:13
thaJeztah and others added 2 commits July 31, 2026 16:14
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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.

Signed-off-by: Paweł Gronowski <git@grono.dev>
@thaJeztah
thaJeztah force-pushed the fix_absolute_symlinks_TEMP branch from f01aca2 to 6c3be27 Compare July 31, 2026 14:16
@thaJeztah

Copy link
Copy Markdown
Member Author

Closing; pushed the commits to #93

@thaJeztah thaJeztah closed this Jul 31, 2026
@thaJeztah
thaJeztah deleted the fix_absolute_symlinks_TEMP branch August 5, 2026 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants