archive: Reject relative escapes before absolute symlinks - #97
Closed
vvoland wants to merge 4 commits into
Closed
Conversation
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>
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>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #97 +/- ##
==========================================
- Coverage 65.81% 65.33% -0.49%
==========================================
Files 42 44 +2
Lines 2039 2325 +286
==========================================
+ Hits 1342 1519 +177
- Misses 519 594 +75
- Partials 178 212 +34 ☔ 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 hardens archive extraction path resolution to ensure relative symlink escapes are rejected even when a later component is an absolute symlink (which would otherwise trigger the resolve-in-root fallback introduced in the stacked PR).
Changes:
- Extend
fsRootPathresolution to record whether a relative escape occurred before encountering an absolute symlink. - Introduce
resolveArchivePath()and apply it toUnpack/UnpackLayerentry paths (and hardlink targets), ensuring consistent chroot-like traversal behavior. - Update implied-directory creation and
unpackedPathstracking to operate on resolved, native-separator, root-relative paths; add regression tests for absolute-symlink traversal and the relative-escape-before-absolute-symlink case.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| rootpath.go | Tracks absolute symlink traversal and relative-escape-before-absolute state during link walking. |
| archive.go | Adds resolveArchivePath() and uses it for extraction paths and hardlink targets; updates implied-dir creation signature/calls. |
| diff.go | Uses resolveArchivePath() in UnpackLayer and aligns unpackedPaths keying with resolved native paths. |
| archive_unix.go | Updates handleLChmod to accept resolved hardlink target paths. |
| archive_windows.go | Updates handleLChmod signature to match the new call shape (no-op behavior unchanged). |
| archive_unix_test.go | Adds regression tests for absolute symlink traversal, relative-escape rejection, and hardlink sources through absolute symlinks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+488
to
+491
| relParent, err := filepath.Rel(root.Name(), resolved.path) | ||
| if err != nil { | ||
| return "", err | ||
| } |
Member
There was a problem hiding this comment.
a relative symlink target itself cannot name another Windows volume. If a target is volume-qualified, it should already satisfy filepath.IsAbs(newpath) and take the absolute-link branch. Therefore a Rel failure here would more likely indicate an unexpected form in the path being inspected than a direct malicious relative target.
Perhaps;
if err != nil {
return "", false, breakoutError(fmt.Errorf("could not resolve symlink %q relative to root: %w", path, err))
}
thaJeztah
reviewed
Jul 31, 2026
| )) | ||
|
|
||
| err := unpacker.unpack(dest, bytes.NewReader(buf.Bytes())) | ||
| assert.Check(t, err != nil, "expected relative symlink escape to be rejected") |
Member
There was a problem hiding this comment.
Perhaps check that it's a breakout path escape error;
assert.Check(t, isPathEscapes(err), "expected path-escape error, got: %v", err)A relative escape in the first path component was resolved by the recursive walk without being recorded. If a later component was an absolute symlink, the fallback treated the original path-escape error as caused by that absolute link and accepted the archive path. 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>
Member
|
closing this one; I included the patch in #93 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A relative escape in the first path component was resolved by the
recursive walk without being recorded. If a later component was an
absolute symlink, the fallback treated the original path-escape error
as caused by that absolute link and accepted the archive path.
Record relative escapes when each symlink is read so a later absolute
link cannot make the path eligible for resolve-in-root fallback.