Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve symlink targets in the local system #8376

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

DavidS
Copy link

@DavidS DavidS commented Sep 7, 2020

This is a 馃悰 bug fix.

Summary

Instead of copying symlinks verbatim to _site, this rewrites them to the absolute path so that they keep pointing at the right file.

Context

This is a rebase of #8299 with all comments from that PR addressed and tests added.

Fixes #6870, closes #8299

@DirtyF DirtyF added the fix label Sep 7, 2020
UnkindPartition and others added 3 commits November 8, 2020 15:04
This is a refresh of the changes by @feuerbach in jekyll#8299, using `FileUtils.ln_sf` to force overwriting instead of calling the `unlink` ourselves.

Fixes jekyll#6870, closes jekyll#8299
Co-authored-by: Michael Currin <18750745+MichaelCurrin@users.noreply.github.com>
@DavidS
Copy link
Author

DavidS commented Nov 8, 2020

Rebased to current HEAD. Anything else I can do to help here?

@ashmaroli
Copy link
Member

Anything else I can do to help here?

:) Thank you for resolving merge conflicts. What this needs is some documentation (and a nice title for this PR.. 馃槈)

Also, waiting for a review from @parkr or @mattr- since symlinks are security concerns..

@DavidS DavidS changed the title Symlinks Preserve symlink targets in the local system Nov 8, 2020
@DirtyF DirtyF added the security label Nov 8, 2020
@mattr-
Copy link
Member

mattr- commented Nov 9, 2020

At a minimum, this needs a check to ensure that the resolved symlink is inside the site's source directory. Dereferencing symlinks that point to outside the site source is a large security risk that this PR doesn't address yet.

@DirtyF DirtyF marked this pull request as draft November 11, 2020 12:33
@DavidS
Copy link
Author

DavidS commented Dec 3, 2020

Oh yeah - I think I can see how that would be awful. Not quite sure how to address though.

My use-case is where I deploy approximately 50GB of files outside of git and want to reference those instead of copying around.

@fauno
Copy link
Member

fauno commented Dec 3, 2020

My use-case is where I deploy approximately 50GB of files outside of git and want to reference those instead of copying around.

Maybe you can use hardlinks instead? :)

@DavidS
Copy link
Author

DavidS commented Dec 6, 2020

Maybe you can use hardlinks instead? :)

This is dependent on another tool (git-annex), which reduces my flexibility a lot.

@fauno
Copy link
Member

fauno commented Dec 7, 2020 via email

@DavidS
Copy link
Author

DavidS commented Mar 7, 2021

Given the issues raised with this draft, I've decided to just eat the additional storage requirements instead of trying to improve on this code here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Symbolic links are broken when being copied to _site
7 participants