Merge pull request #4710 from fenollp/safe-symlinks

Merge pull request 4710

lib/jekyll/collection.rb

@@ -94,7 +94,7 @@ module Jekyll
         Dir.chdir(directory) do
           entry_filter.filter(entries).reject do |f|
             path = collection_dir(f)
-            File.directory?(path) || (File.symlink?(f) && site.safe)
+            File.directory?(path) || entry_filter.symlink?(f)
           end
         end
     end
@@ -135,7 +135,7 @@ module Jekyll
     # Returns false if the directory doesn't exist or if it's a symlink
     #   and we're in safe mode.
     def exists?
-      File.directory?(directory) && !(File.symlink?(directory) && site.safe)
+      File.directory?(directory) && !entry_filter.symlink?(directory)
     end
 
     # The entry filter for this collection.

lib/jekyll/entry_filter.rb

@@ -52,7 +52,11 @@ module Jekyll
     end
 
     def symlink?(entry)
-      File.symlink?(entry) && site.safe
+      site.safe && File.symlink?(entry) && symlink_outside_site_source?(entry)
+    end
+
+    def symlink_outside_site_source?(entry)
+      ! File.realpath(entry).start_with?(File.realpath(@site.source))
     end
 
     def ensure_leading_slash(path)

lib/jekyll/readers/data_reader.rb

@@ -4,6 +4,7 @@ module Jekyll
     def initialize(site)
       @site = site
       @content = {}
+      @entry_filter = EntryFilter.new(site)
     end
 
     # Read all the files in <source>/<dir>/_drafts and create a new Draft
@@ -26,7 +27,7 @@ module Jekyll
     #
     # Returns nothing
     def read_data_to(dir, data)
-      return unless File.directory?(dir) && (!site.safe || !File.symlink?(dir))
+      return unless File.directory?(dir) && !@entry_filter.symlink?(dir)
 
       entries = Dir.chdir(dir) do
         Dir['*.{yaml,yml,json,csv}'] + Dir['*'].select { |fn| File.directory?(fn) }
@@ -34,7 +35,7 @@ module Jekyll
 
       entries.each do |entry|
         path = @site.in_source_dir(dir, entry)
-        next if File.symlink?(path) && site.safe
+        next if @entry_filter.symlink?(path)
 
         key = sanitize_filename(File.basename(entry, '.*'))
         if File.directory?(path)

test/test_collections.rb

@@ -179,13 +179,13 @@ class TestCollections < JekyllUnitTest
       @collection = @site.collections["methods"]
     end
 
-    should "not allow symlinks" do
-      refute_includes @collection.filtered_entries, "um_hi.md"
+    should "include the symlinked file as it resolves to inside site.source" do
+      assert_includes @collection.filtered_entries, "um_hi.md"
       refute_includes @collection.filtered_entries, "/um_hi.md"
     end
 
-    should "not include the symlinked file in the list of docs" do
-      refute_includes @collection.docs.map(&:relative_path), "_methods/um_hi.md"
+    should "include the symlinked file in the list of docs as it resolves to inside site.source" do
+      assert_includes @collection.docs.map(&:relative_path), "_methods/um_hi.md"
     end
   end
 

test/test_entry_filter.rb

@@ -46,11 +46,11 @@ class TestEntryFilter < JekyllUnitTest
       assert_equal files, @site.reader.filter_entries(files)
     end
 
-    should "filter symlink entries when safe mode enabled" do
+    should "keep safe symlink entries when safe mode enabled" do
       site = Site.new(site_configuration('safe' => true))
       allow(File).to receive(:symlink?).with('symlink.js').and_return(true)
       files = %w[symlink.js]
-      assert_equal [], site.reader.filter_entries(files)
+      assert_equal files, @site.reader.filter_entries(files)
     end
 
     should "not filter symlink entries when safe mode disabled" do
@@ -59,12 +59,18 @@ class TestEntryFilter < JekyllUnitTest
       assert_equal files, @site.reader.filter_entries(files)
     end
 
-    should "not include symlinks in safe mode" do
+    should "filter symlink pointing outside site source" do
+      ent1 = %w[_includes/tmp]
+      entries = EntryFilter.new(@site).filter(ent1)
+      assert_equal %w[], entries
+    end
+
+    should "include only safe symlinks in safe mode" do
       site = Site.new(site_configuration('safe' => true))
 
       site.reader.read_directories("symlink-test")
-      assert_equal [], site.pages
-      assert_equal [], site.static_files
+      assert_equal %w[main.scss symlinked-file].length, site.pages.length
+      refute_equal [], site.static_files
     end
 
     should "include symlinks in unsafe mode" do

test/test_site.rb

@@ -418,12 +418,12 @@ class TestSite < JekyllUnitTest
         assert_equal site.site_payload['site']['data']['products'], file_content
       end
 
-      should "not load symlink files in safe mode" do
+      should "load the symlink files in safe mode, as they resolve to inside site.source" do
         site = Site.new(site_configuration('safe' => true))
         site.process
-
-        assert_nil site.data['products']
-        assert_nil site.site_payload['site']['data']['products']
+        file_content = SafeYAML.load_file(File.join(source_dir, '_data', 'products.yml'))
+        assert_equal site.data['products'], file_content
+        assert_equal site.site_payload['site']['data']['products'], file_content
       end
 
     end