Merge pull request #256 from sodabrew/wip-safe-yaml

Use Psych and Safe YAML for reports processing

.travis.yml

@@ -9,6 +9,7 @@ services:
 rvm:
   - "1.8.7"
   - "1.9.3"
+  - "2.0.0"
 before_script:
   - cp config/database.yml.travis config/database.yml
   - 'echo "test: *${DB}" >> config/database.yml'

Gemfile

@@ -10,6 +10,7 @@ gem 'rack'
 
 gem 'daemons'
 gem 'json_pure'
+gem 'safe_yaml',    :require => false
 gem 'fastercsv',    :platforms => :ruby_18
 gem 'system_timer', :platforms => :ruby_18
 

Gemfile.lock

@@ -120,6 +120,7 @@ GEM
       rspec-core (~> 2.13.0)
       rspec-expectations (~> 2.13.0)
       rspec-mocks (~> 2.13.0)
+    safe_yaml (1.0.0)
     sass (3.2.13)
     sass-rails (3.2.6)
       railties (~> 3.2.0)
@@ -175,6 +176,7 @@ DEPENDENCIES
   rake (~> 0.9.3)
   rspec-html-matchers
   rspec-rails (~> 2.13.0)
+  safe_yaml
   sass-rails (~> 3.2)
   shoulda-matchers (< 2.0)
   sqlite3

app/models/node.rb

@@ -114,6 +114,15 @@ def configuration
     }
   end
 
+  # Psych YAML calls this method
+  def encode_with(coder)
+    coder.tag = nil
+    configuration.each do |k, v|
+      coder[k] = v
+    end
+  end
+
+  # Syck YAML calls this method
   def to_yaml(opts={})
     configuration.to_yaml(opts)
   end

app/models/report.rb

@@ -101,30 +101,20 @@ def self.remove_file(file)
 
   def self.create_from_yaml_file(report_file, options = {})
     report = create_from_yaml(read_file_contents(report_file))
-    remove_file(report_file) if options[:delete]
-    return report
-  rescue Exception => e
-    retries ||= 3
-    retry if (retries -= 1) > 0
-    DelayedJobFailure.create!(
-      :summary   => "Importing report #{File.basename(report_file)}",
-      :details   => e.to_s,
-      :backtrace => Rails.backtrace_cleaner.clean(e.backtrace)
-    )
-    return nil
+    remove_file(report_file) if options[:delete] && report
+    report
   end
 
   def self.create_from_yaml(report_yaml)
-    raw_report = YAML.load(report_yaml)
+    raw_report = YAML.load(report_yaml, :safe => :true, :deserialize_symbols => true)
 
-    unless raw_report.is_a? Puppet::Transaction::Report
-      raise ArgumentError, "The supplied report is in invalid format '#{raw_report.class}', expected 'Puppet::Transaction::Report'"
+    unless raw_report.is_a? Hash
+      raise ArgumentError, 'The supplied report did not deserialize into a Hash'
     end
 
-    raw_report.extend(ReportExtensions)
-    report_hash = ReportTransformer.apply(raw_report.to_hash)
+    report_hash = ReportTransformer.apply(ReportSanitizer.sanitize(raw_report))
 
-    report_hash["resource_statuses"] = report_hash["resource_statuses"].values
+    report_hash['resource_statuses'] = report_hash['resource_statuses'].values
 
     report = Report.new(Report.attribute_hash_from(report_hash)).munge
 
@@ -135,7 +125,24 @@ def self.create_from_yaml(report_yaml)
     end
 
     report.save!
-    report
+    return report
+  rescue => e
+    retries ||= 3
+    retry if (retries -= 1) > 0
+    DelayedJobFailure.create!(
+      :summary   => "Importing report",
+      :details   => e.to_s,
+      :backtrace => Rails.backtrace_cleaner.clean(e.backtrace)
+    )
+    return nil
+  end
+
+  def self.read_file_contents(file)
+    File.read(file)
+  end
+
+  def self.remove_file(file)
+    File.unlink(file)
   end
 
   def assign_to_node

config/boot.rb

@@ -1,10 +1,5 @@
 require 'rubygems'
 
-# Puppet reports do not work with psych. Must set this prior to loading
-# the Gemfile, otherwise delayed_job, etc. would use psych.
-require 'yaml'
-YAML::ENGINE.yamler = 'syck' if RUBY_VERSION >= '1.9'
-
 # Set up gems listed in the Gemfile.
 ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../../Gemfile', __FILE__)
 

config/initializers/requires.rb

@@ -1,7 +1,7 @@
 # Local libraries
 require "#{Rails.root}/lib/has_parameters"
-require "#{Rails.root}/lib/puppet/report"
 require "#{Rails.root}/lib/csv_extensions"
 require "#{Rails.root}/lib/add_renderers"
 require "#{Rails.root}/lib/registry"
 require "#{Rails.root}/lib/core_callbacks"
+require "#{Rails.root}/lib/puppet/report_sanitizer"

config/initializers/safe_yaml.rb

@@ -0,0 +1,9 @@
+require 'safe_yaml'
+
+# Set default for YAML.load to unsafe so we don't affect performance
+# unnecessarily -- we call it safely explicitly where needed
+SafeYAML::OPTIONS[:default_mode] = :unsafe
+
+# Whitelist Symbol objects
+SafeYAML::OPTIONS[:whitelisted_tags] << 'tag:ruby.yaml.org,2002:sym' #syck
+SafeYAML::OPTIONS[:whitelisted_tags] << '!ruby/sym' #psych

db/migrate/20101206225510_schematize_reports.rb

@@ -1,15 +1,5 @@
 require "#{Rails.root}/lib/progress_bar"
 
-class Report < ActiveRecord::Base
-  serialize :report, Puppet::Transaction::Report
-
-  def report
-    rep = read_attribute(:report)
-    rep.extend(ReportExtensions) unless rep.nil? or rep.is_a? ReportExtensions
-    rep
-  end
-end
-
 class SchematizeReports < ActiveRecord::Migration
   def self.up
     create_table :report_logs do |t|