3.7.x: EntryFilter#filter symlink fix #7224
Conversation
Previously, you could include the name of a symlinked file and Jekyll would not filter it. This is considered a bypass of the symlink checking, and thus a security bug.
lib/jekyll/entry_filter.rb
Outdated
| special?(e) || backup?(e) || excluded?(e) || symlink?(e) | ||
| end | ||
| next true if symlink?(e) | ||
| next false if included?(e) |
There was a problem hiding this comment.
@parkr For knowledge purposes, can you pleas explain what these two lines mean..?
Thanks =)
There was a problem hiding this comment.
We first check for symlinks and directly go to next entry if true
test/test_entry_filter.rb
Outdated
| # no support for symlinks on Windows | ||
| skip_if_windows "Jekyll does not currently support symlinks on Windows." | ||
|
|
||
| site = Site.new(site_configuration("safe" => true, "include" => ["symlinked-file-outside-source"])) |
There was a problem hiding this comment.
We have a helper for this.. fixture_site
site = fixture_site(
:safe => true,
:include => ["filename"]
)
test/test_layout_reader.rb
Outdated
| should "only read the layouts which are in the site" do | ||
| layouts = LayoutReader.new(@site).read | ||
|
|
||
| refute layouts.keys.include?("symlink"), "Should not read the symlinked layout" |
There was a problem hiding this comment.
Prefer using Hash#include? or Hash#key?` instead..?
|
@pathawks is Utterson preventing Travis from running tests here? |
|
@DirtyF I can't imagine why that would be the case 🤷♂️ |
|
I don't see why Travis / AppVeyor aren't working... I ran |
|
Looks like Checks API is waiting for Utterson to complete before launching Travis CI tests. 🤔 Tests are also passing locally on my end. |
Where do you see this? |
|
@parkr It's just a guess based on what is displayed on https://github.com/jekyll/jekyll/pull/7224/checks if Utterson is queued (and is blocked because of EC2), I conclude that Travis tests will not start until first job is finished. |
|
@parkr Thank you for addressing my concerns.. |
|
@ashmaroli I'll make a new PR to |
This reverts commit f5cd15c.
In
EntryFilter#filter, we check forinclude?before we check forsymlink?, which allows symlinks to be read in a build when they shouldn't be by just including them.I am committing a failing test in one commit, and a fix in another commit so we can demonstrate what fails.