From 480e35037bfc058819709209d528b602c838c6ea Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Wed, 23 Oct 2013 10:55:48 -0400 Subject: [PATCH 01/12] A start at a gem-based plugin whitelist for Pages. --- lib/jekyll/site.rb | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/lib/jekyll/site.rb b/lib/jekyll/site.rb index b892a31a..cbd7eba7 100644 --- a/lib/jekyll/site.rb +++ b/lib/jekyll/site.rb @@ -77,11 +77,10 @@ module Jekyll require f end end - self.gems.each do |gem| - require gem - end end + require_gems + self.converters = instantiate_subclasses(Jekyll::Converter) self.generators = instantiate_subclasses(Jekyll::Generator) end @@ -97,6 +96,18 @@ module Jekyll end end + def require_gems + self.gems.each do |gem| + if gem_whitelist.include?(gem) || !self.safe + require gem + end + end + end + + def gem_whitelist + @gem_whitelist ||= [] + end + # Internal: Setup the plugin search path # # Returns an Array of plugin search paths From a4b9bab1dce7ea58eea878b1bc38b7ddcdbefdb0 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Fri, 25 Oct 2013 13:25:44 -0400 Subject: [PATCH 02/12] Add --whitelist flag and internal logic --- bin/jekyll | 1 + lib/jekyll/site.rb | 13 +++++++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/bin/jekyll b/bin/jekyll index 50d10e3a..03c37b8b 100755 --- a/bin/jekyll +++ b/bin/jekyll @@ -41,6 +41,7 @@ def add_build_options(c) c.option '--lsi', 'Use LSI for improved related posts' c.option '-D', '--drafts', 'Render posts in the _drafts folder' c.option '-V', '--verbose', 'Print verbose output.' + c.option '-W', '--whitelist', 'Gem plugin whitelist' end command :default do |c| diff --git a/lib/jekyll/site.rb b/lib/jekyll/site.rb index cbd7eba7..2ba3639b 100644 --- a/lib/jekyll/site.rb +++ b/lib/jekyll/site.rb @@ -3,7 +3,8 @@ module Jekyll attr_accessor :config, :layouts, :posts, :pages, :static_files, :categories, :exclude, :include, :source, :dest, :lsi, :pygments, :permalink_style, :tags, :time, :future, :safe, :plugins, :limit_posts, - :show_drafts, :keep_files, :baseurl, :data, :file_read_opts, :gems + :show_drafts, :keep_files, :baseurl, :data, :file_read_opts, + :gems, :whitelist attr_accessor :converters, :generators @@ -98,14 +99,18 @@ module Jekyll def require_gems self.gems.each do |gem| - if gem_whitelist.include?(gem) || !self.safe + if whitelist.include?(gem) || !self.safe require gem end end end - def gem_whitelist - @gem_whitelist ||= [] + def whitelist + @whitelist ||= begin + YAML.safe_load_file(self.config['whitelist']) || [] + rescue + [] + end end # Internal: Setup the plugin search path From 615d49ed664fb117540fac6ca8aafbf3cb7647e8 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Fri, 25 Oct 2013 13:36:04 -0400 Subject: [PATCH 03/12] Remove Site.whitelist attribute. --- lib/jekyll/site.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/jekyll/site.rb b/lib/jekyll/site.rb index 2ba3639b..e898647d 100644 --- a/lib/jekyll/site.rb +++ b/lib/jekyll/site.rb @@ -3,8 +3,7 @@ module Jekyll attr_accessor :config, :layouts, :posts, :pages, :static_files, :categories, :exclude, :include, :source, :dest, :lsi, :pygments, :permalink_style, :tags, :time, :future, :safe, :plugins, :limit_posts, - :show_drafts, :keep_files, :baseurl, :data, :file_read_opts, - :gems, :whitelist + :show_drafts, :keep_files, :baseurl, :data, :file_read_opts, :gems attr_accessor :converters, :generators From 6187861e911eb624434e1fd77a9a8696264d1fe8 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Fri, 6 Dec 2013 00:21:34 -0500 Subject: [PATCH 04/12] Add test code for 'whitelist' option. --- features/site_configuration.feature | 23 +++++++++++++++++++++++ features/step_definitions/jekyll_steps.rb | 4 ++++ features/support/env.rb | 3 ++- 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/features/site_configuration.feature b/features/site_configuration.feature index 4f807eae..6ca59601 100644 --- a/features/site_configuration.feature +++ b/features/site_configuration.feature @@ -233,3 +233,26 @@ Feature: Site configuration Then the _site directory should exist And I should see "Whatever" in "_site/index.html" And I should see "this is a test" in "_site/test.txt" + + Scenario: Add an empty whitelist to restrict all gems + Given I have an "index.html" file that contains "Whatever" + And I have a configuration file with: + | key | value | + | gems | [jekyll_test_plugin] | + | whitelist | [] | + When I run jekyll in safe mode + Then the _site directory should exist + And I should see "Whatever" in "_site/index.html" + And the "_site/test.txt" file should not exist + + Scenario: Add a whitelist to restrict some gems but allow others + Given I have an "index.html" file that contains "Whatever" + And I have a configuration file with: + | key | value | + | gems | [jekyll_test_plugin, jekyll_test_plugin_malicious] | + | whitelist | [jekyll_test_plugin] | + When I run jekyll in safe mode + Then the _site directory should exist + And I should see "Whatever" in "_site/index.html" + And the "_site/test.txt" file should exist + And I should see "this is a test" in "_site/test.txt" diff --git a/features/step_definitions/jekyll_steps.rb b/features/step_definitions/jekyll_steps.rb index 7a74486b..5f8f93fc 100644 --- a/features/step_definitions/jekyll_steps.rb +++ b/features/step_definitions/jekyll_steps.rb @@ -126,6 +126,10 @@ When /^I run jekyll$/ do run_jekyll end +When /^I run jekyll in safe mode$/ do + run_jekyll(:safe => true) +end + When /^I run jekyll with drafts$/ do run_jekyll(:drafts => true) end diff --git a/features/support/env.rb b/features/support/env.rb index 5ccbab98..5fcacc8a 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -15,6 +15,7 @@ def run_jekyll(opts = {}) command = JEKYLL_PATH.clone command << " build" command << " --drafts" if opts[:drafts] + command << " --safe" if opts[:safe] command << " >> /dev/null 2>&1" if opts[:debug].nil? system command end @@ -50,7 +51,7 @@ def seconds_agnostic_datetime(datetime = Time.now) pieces = datetime.to_s.split(" ") if pieces.size == 6 # Ruby 1.8.7 date = pieces[0..2].join(" ") - time = seconds_agnostic_time(pieces[3]) + time = seconds_agnostic_time(pieces[3]) zone = pieces[4..5].join(" ") else # Ruby 1.9.1 or greater date, time, zone = pieces From be5966a2ec63a3bc7ae1bd4f4dba3cc8120be4b1 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Fri, 6 Dec 2013 00:21:47 -0500 Subject: [PATCH 05/12] Remove option from CLI --- bin/jekyll | 1 - 1 file changed, 1 deletion(-) diff --git a/bin/jekyll b/bin/jekyll index 03c37b8b..50d10e3a 100755 --- a/bin/jekyll +++ b/bin/jekyll @@ -41,7 +41,6 @@ def add_build_options(c) c.option '--lsi', 'Use LSI for improved related posts' c.option '-D', '--drafts', 'Render posts in the _drafts folder' c.option '-V', '--verbose', 'Print verbose output.' - c.option '-W', '--whitelist', 'Gem plugin whitelist' end command :default do |c| From 8d898dafde3d4624fb5ad9552571c21b3e1ca7cb Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Fri, 6 Dec 2013 00:22:07 -0500 Subject: [PATCH 06/12] Add "jekyll_test_plugin_malicious" as gem dep --- jekyll.gemspec | 1 + 1 file changed, 1 insertion(+) diff --git a/jekyll.gemspec b/jekyll.gemspec index 62a866a6..4aeab9b8 100644 --- a/jekyll.gemspec +++ b/jekyll.gemspec @@ -49,6 +49,7 @@ Gem::Specification.new do |s| s.add_development_dependency('mime-types', "~> 1.5") s.add_development_dependency('activesupport', '~> 3.2.13') s.add_development_dependency('jekyll_test_plugin') + s.add_development_dependency('jekyll_test_plugin_malicious') # = MANIFEST = s.files = %w[ From 5c9f9a4de8ac23d10c498f2fead7720f2df141bc Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Fri, 6 Dec 2013 00:23:22 -0500 Subject: [PATCH 07/12] The whitelist should be an array (not a file) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Default to `[]` if the key’s value is falsey --- lib/jekyll/site.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/jekyll/site.rb b/lib/jekyll/site.rb index e898647d..d390eb12 100644 --- a/lib/jekyll/site.rb +++ b/lib/jekyll/site.rb @@ -106,7 +106,7 @@ module Jekyll def whitelist @whitelist ||= begin - YAML.safe_load_file(self.config['whitelist']) || [] + Array[self.config['whitelist']].flatten || [] rescue [] end From 249b13a98a58b48752e1ce8314ac6aab8a21401c Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Fri, 6 Dec 2013 00:24:59 -0500 Subject: [PATCH 08/12] Hm, shouldn't need that call to 'rescue' in Site#whitelist --- lib/jekyll/site.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/jekyll/site.rb b/lib/jekyll/site.rb index d390eb12..cc029ecb 100644 --- a/lib/jekyll/site.rb +++ b/lib/jekyll/site.rb @@ -105,11 +105,7 @@ module Jekyll end def whitelist - @whitelist ||= begin - Array[self.config['whitelist']].flatten || [] - rescue - [] - end + @whitelist ||= Array[self.config['whitelist']].flatten || [] end # Internal: Setup the plugin search path From f0fbc8f356086f5dd53fb03a274cfdc25ebb76b3 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Fri, 6 Dec 2013 00:26:44 -0500 Subject: [PATCH 09/12] Refactor conditions for allowing plugins into a new method: Site#plguin_allowed? --- lib/jekyll/site.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/jekyll/site.rb b/lib/jekyll/site.rb index cc029ecb..12a75ebf 100644 --- a/lib/jekyll/site.rb +++ b/lib/jekyll/site.rb @@ -98,12 +98,16 @@ module Jekyll def require_gems self.gems.each do |gem| - if whitelist.include?(gem) || !self.safe + if plugin_allowed?(gem) require gem end end end + def plugin_allowed?(name) + whitelist.include?(gem_name) || !self.safe + end + def whitelist @whitelist ||= Array[self.config['whitelist']].flatten || [] end From bce2c2efb41fd128195256455dd9714183abb124 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Fri, 6 Dec 2013 00:52:33 -0500 Subject: [PATCH 10/12] Print the output of Jekyll if the command fails --- features/step_definitions/jekyll_steps.rb | 16 +++++++---- features/support/env.rb | 34 ++++++++++++++--------- 2 files changed, 31 insertions(+), 19 deletions(-) diff --git a/features/step_definitions/jekyll_steps.rb b/features/step_definitions/jekyll_steps.rb index 5f8f93fc..43c7e31d 100644 --- a/features/step_definitions/jekyll_steps.rb +++ b/features/step_definitions/jekyll_steps.rb @@ -1,9 +1,13 @@ Before do - FileUtils.rm_rf(TEST_DIR) FileUtils.mkdir(TEST_DIR) Dir.chdir(TEST_DIR) end +After do + FileUtils.rm_rf(TEST_DIR) + FileUtils.rm_rf(JEKYLL_COMMAND_OUTPUT_FILE) +end + World(Test::Unit::Assertions) Given /^I have a blank site in "(.*)"$/ do |path| @@ -123,23 +127,23 @@ end When /^I run jekyll$/ do - run_jekyll + run_jekyll_build end When /^I run jekyll in safe mode$/ do - run_jekyll(:safe => true) + run_jekyll_build("--safe") end When /^I run jekyll with drafts$/ do - run_jekyll(:drafts => true) + run_jekyll_build("--drafts") end When /^I call jekyll new with test_blank --blank$/ do - call_jekyll_new(:path => "test_blank", :blank => true) + run_jekyll_new("test_blank --blank") end When /^I debug jekyll$/ do - run_jekyll(:debug => true) + run_jekyll_build("--verbose") end When /^I change "(.*)" to contain "(.*)"$/ do |file, text| diff --git a/features/support/env.rb b/features/support/env.rb index 5fcacc8a..c6d47583 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -10,23 +10,31 @@ require 'time' TEST_DIR = File.join('/', 'tmp', 'jekyll') JEKYLL_PATH = File.join(File.dirname(__FILE__), '..', '..', 'bin', 'jekyll') +JEKYLL_COMMAND_OUTPUT_FILE = File.join('/', 'tmp', 'jekyll_output.txt') -def run_jekyll(opts = {}) - command = JEKYLL_PATH.clone - command << " build" - command << " --drafts" if opts[:drafts] - command << " --safe" if opts[:safe] - command << " >> /dev/null 2>&1" if opts[:debug].nil? +def jekyll_output_file + JEKYLL_COMMAND_OUTPUT_FILE +end + +def jekyll_output + File.read(jekyll_output_file) +end + +def run_jekyll(args, output_file) + command = "#{JEKYLL_PATH} #{args} > #{jekyll_output_file} 2>&1" system command end -def call_jekyll_new(opts = {}) - command = JEKYLL_PATH.clone - command << " new" - command << " #{opts[:path]}" if opts[:path] - command << " --blank" if opts[:blank] - command << " >> /dev/null 2>&1" if opts[:debug].nil? - system command +def run_jekyll_build(build_args = "") + if !run_jekyll("build #{build_args}", jekyll_output_file) || build_args.eql?("--verbose") + puts jekyll_run_output + end +end + +def run_jekyll_new(new_args = "") + unless run_jekyll("new #{new_args}", jekyll_output_file) + puts jekyll_run_output + end end def slug(title) From 5591ff2a6bcd4895e334d9f35e344463473b832d Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Fri, 6 Dec 2013 00:57:19 -0500 Subject: [PATCH 11/12] Whoops, it's called 'jekyll_run_output' --- features/support/env.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/support/env.rb b/features/support/env.rb index c6d47583..cf2055a1 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -16,7 +16,7 @@ def jekyll_output_file JEKYLL_COMMAND_OUTPUT_FILE end -def jekyll_output +def jekyll_run_output File.read(jekyll_output_file) end From e91db82d2620e2981691ec49d9a0add20d8d557b Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Fri, 6 Dec 2013 01:07:34 -0500 Subject: [PATCH 12/12] I mean the argument and the local variable should be the same thing --- lib/jekyll/site.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/jekyll/site.rb b/lib/jekyll/site.rb index 12a75ebf..085bfaea 100644 --- a/lib/jekyll/site.rb +++ b/lib/jekyll/site.rb @@ -104,7 +104,7 @@ module Jekyll end end - def plugin_allowed?(name) + def plugin_allowed?(gem_name) whitelist.include?(gem_name) || !self.safe end