fix: overwrite symlinks when extracting tarballs #867
Conversation
Wwwsylvia
requested review from
sajayantony,
shizhMSFT and
SteveLasker
as code owners
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #867 +/- ##
==========================================
+ Coverage 75.67% 76.03% +0.35%
==========================================
Files 63 63
Lines 6011 6025 +14
==========================================
+ Hits 4549 4581 +32
+ Misses 1079 1064 -15
+ Partials 383 380 -3 ☔ View full report in Codecov by Sentry. |
| @@ -61,6 +61,7 @@ func tarDirectory(ctx context.Context, root, prefix string, w io.Writer, removeT | |||
| name = filepath.ToSlash(name) | |||
|
|
|||
| // Generate header | |||
| // NOTE: We don't support hard links and treat it as regular files | |||
There was a problem hiding this comment.
If there wqs a duplicate blob in this with the same shasum, a hard link would be ideal. Is that a possibility with this method?
There was a problem hiding this comment.
Do you mean users may push a directory containing two files, one of them is a hark link to another?
| // NOTE: ORAS does not generate hard links when creating tarballs. | ||
| // If a hard link is found in the tarball, it will be extracted. | ||
| // If the target link already exists, os.Link will throw an error. | ||
| // This is a known limitation and will not be addressed. |
There was a problem hiding this comment.
If supporting links is possible and maybe desirable in the futre, maybe just make an issue out of this
There was a problem hiding this comment.
Hard links in tarballs are tricky to support and can potentially introduce issues. For example, it requires exhaustive handling/tests on different OS. If there is a valid use case, we can evaluate the scenario and see if we need an issue for this.
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
Wwwsylvia
force-pushed
the
overwrite_file_links
branch
from
62aba2f to
9fb9f9e
Compare
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
| @@ -166,29 +167,41 @@ | |||
| } | |||
|
|
|||
| // Name check | |||
| name := header.Name | |||
| path, err := ensureBasePath(dir, prefix, name) | |||
| filename := header.Name | |||
Check failure
Code scanning / CodeQL
Arbitrary file write extracting an archive containing symbolic links High
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
content/file/utils.go
Outdated
| if _, err := os.Lstat(filePath); err == nil { | ||
| // link already exists, remove it first | ||
| if err := os.Remove(filePath); err != nil { | ||
| return err | ||
| } | ||
| } | ||
| err = os.Symlink(target, filePath) |
There was a problem hiding this comment.
We should assume the link does not exist. If os.Symlink() fails, we should then remove and retry.
There was a problem hiding this comment.
This makes sense, updated.
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
| } | ||
| case tar.TypeSymlink: | ||
| var target string | ||
| if target, err = ensureLinkPath(dir, prefix, path, header.Linkname); err == nil { | ||
| err = os.Symlink(target, path) | ||
| target, err = ensureLinkPath(dirPath, dirName, filePath, header.Linkname) |
Check failure
Code scanning / CodeQL
Arbitrary file write extracting an archive containing symbolic links High
Fixes #865