diff --git a/lib/inspec/formatters/base.rb b/lib/inspec/formatters/base.rb index 436da5147..d3f2ecf92 100644 --- a/lib/inspec/formatters/base.rb +++ b/lib/inspec/formatters/base.rb @@ -158,6 +158,7 @@ module Inspec::Formatters start_time: example.execution_result.started_at.to_datetime.rfc3339.to_s, resource_title: example.metadata[:described_class] || example.metadata[:example_group][:description], expectation_message: format_expectation_message(example), + waiver_data: example.metadata[:waiver_data], } unless (pid = example.metadata[:profile_id]).nil? diff --git a/lib/inspec/rule.rb b/lib/inspec/rule.rb index 66d6e204c..c46a1172b 100644 --- a/lib/inspec/rule.rb +++ b/lib/inspec/rule.rb @@ -1,6 +1,7 @@ # copyright: 2015, Dominik Richter require "method_source" +require "date" require "inspec/describe" require "inspec/expect" require "inspec/resource" @@ -28,6 +29,7 @@ module Inspec @resource_dsl end + attr_reader :__waiver_data def initialize(id, profile_id, opts, &block) @impact = nil @title = nil @@ -42,7 +44,7 @@ module Inspec @__rule_id = id @__profile_id = profile_id @__checks = [] - @__skip_rule = {} + @__skip_rule = {} # { result: true, message: "Why", type: [:only_if, :waiver] } @__merge_count = 0 @__merge_changes = [] @__skip_only_if_eval = opts[:skip_only_if_eval] @@ -52,6 +54,11 @@ module Inspec begin instance_eval(&block) + + # By applying waivers *after* the instance eval, we assure that + # waivers have higher precedence than only_if. + __apply_waivers + rescue StandardError => e # We've encountered an exception while trying to eval the code inside the # control block. We need to prevent the exception from bubbling up, and @@ -141,6 +148,7 @@ module Inspec return if @__skip_only_if_eval == true @__skip_rule[:result] ||= !yield + @__skip_rule[:type] = :only_if @__skip_rule[:message] = message end @@ -193,9 +201,9 @@ module Inspec rule.instance_variable_get(:@__skip_rule) end - def self.set_skip_rule(rule, value, message = nil) + def self.set_skip_rule(rule, value, message = nil, type = :only_if) rule.instance_variable_set(:@__skip_rule, - { result: value, message: message }) + { result: value, message: message, type: type }) end def self.merge_count(rule) @@ -206,14 +214,16 @@ module Inspec rule.instance_variable_get(:@__merge_changes) end + # If a rule is marked to be skipped, this + # creates a dummay array of "checks" with a skip outcome def self.prepare_checks(rule) skip_check = skip_status(rule) return checks(rule) unless skip_check[:result].eql?(true) if skip_check[:message] - msg = "Skipped control due to only_if condition: #{skip_check[:message]}" + msg = "Skipped control due to #{skip_check[:type]} condition: #{skip_check[:message]}" else - msg = "Skipped control due to only_if condition." + msg = "Skipped control due to #{skip_check[:type]} condition." end # TODO: we use os as the carrier here, but should consider @@ -251,7 +261,8 @@ module Inspec skip_check = skip_status(src) sr = skip_check[:result] msg = skip_check[:message] - set_skip_rule(dst, sr, msg) unless sr.nil? + skip_type = skip_check[:type] + set_skip_rule(dst, sr, msg, skip_type) unless sr.nil? # Save merge history dst.instance_variable_set(:@__merge_count, merge_count(dst) + 1) @@ -267,6 +278,56 @@ module Inspec @__checks.push([describe_or_expect, values, block]) end + # Look for an input with a matching ID, and if found, apply waiver + # skipping logic. Basically, if we have a current waiver, and it says + # to skip, we'll replace all the checks with a dummy check (same as + # only_if mechanism) + # Double underscore: not intended to be called as part of the DSL + def __apply_waivers + input_name = @__rule_id # TODO: control ID slugging + registry = Inspec::InputRegistry.instance + input = registry.inputs_by_profile.dig(@__profile_id, input_name) + return unless input + + # An InSpec Input is a datastructure that tracks a profile parameter + # over time. Its value can be set by many sources, and it keeps a + # log of each "set" event so that when it is collapsed to a value, + # it can determine the correct (highest priority) value. + # Store in an instance variable for.. later reading??? + @__waiver_data = input.value + __waiver_data["skipped_due_to_waiver"] = false + __waiver_data["message"] = "" + + # Waivers should have a hash value with keys possibly including skip and + # expiration_date. We only care here if it has a skip key and it + # is yes-like, since all non-skipped waiver operations are handled + # during reporting phase. + return unless __waiver_data.key?("skip") && __waiver_data["skip"] + + # OK, the intent is to skip. Does it have an expiration date, and + # if so, is it in the future? + expiry = __waiver_data["expiration_date"] + if expiry + if expiry.is_a?(Date) + # It appears that yaml.rb automagically parses dates for us + if expiry < Date.today # If the waiver expired, return - no skip applied + __waiver_data["message"] = "Waiver expired on #{expiry}, evaluating control normally" + return + end + else + ui = Inspec::UI.new + ui.error("Unable to parse waiver expiration date '#{expiry}' for control #{@__rule_id}") + ui.exit(:usage_error) + end + end + + # OK, apply a skip. + @__skip_rule[:result] = true + @__skip_rule[:type] = :waiver + @__skip_rule[:message] = __waiver_data["justification"] + __waiver_data["skipped_due_to_waiver"] = true + end + # # Takes a block and returns a block that will run the given block # with access to the resource_dsl of the current class. This is to diff --git a/lib/inspec/runner_rspec.rb b/lib/inspec/runner_rspec.rb index d136b99bf..ffa6963a1 100644 --- a/lib/inspec/runner_rspec.rb +++ b/lib/inspec/runner_rspec.rb @@ -171,6 +171,7 @@ module Inspec metadata[:descriptions] = rule.descriptions metadata[:code] = rule.instance_variable_get(:@__code) metadata[:source_location] = rule.instance_variable_get(:@__source_location) + metadata[:waiver_data] = rule.__waiver_data end end end diff --git a/test/functional/waivers_test.rb b/test/functional/waivers_test.rb new file mode 100644 index 000000000..273b208bf --- /dev/null +++ b/test/functional/waivers_test.rb @@ -0,0 +1,104 @@ +require "functional/helper" + +describe "waivers" do + include FunctionalHelper + let(:waivers_profiles_path) { "#{profile_path}/waivers" } + let(:run_result) { run_inspec_process(cmd, json: true) } + let(:controls_by_id) { run_result.payload.json.dig("profiles", 0, "controls").map { |c| [c["id"], c] }.to_h } + let(:cmd) { "exec #{waivers_profiles_path}/#{profile_name} --input-file #{waivers_profiles_path}/#{profile_name}/files/#{waiver_file}" } + + def assert_test_outcome(expected, control_id) + assert_equal "#{control_id}_#{expected}", "#{control_id}_#{controls_by_id.dig(control_id, "results", 0, "status")}" + end + + def assert_waiver_annotation(control_id) + # TODO - test JSON for waiver annotation + end + + def refute_waiver_annotation(control_id) + # TODO - test JSON for waiver annotation + # Don't suppose we get this for free by defining assert_waiver_annotation ... + end + + describe "a fully pre-slugged control file" do + let(:profile_name) { "basic" } + let(:waiver_file) { "waivers.yaml" } + + it "has all of the expected outcomes" do + [ + "01_not_waivered_passes", + "03_waivered_no_expiry_not_skipped_passes", # this had a waiver but still passed - no annotation? + "06_waivered_expiry_in_past_not_skipped_passes", # a stale waiver + "08_waivered_expiry_in_past_skipped", # another stale waiver + "09_waivered_expiry_in_future_not_skipped_passes", # unneeded waiver + ].each do |control_id| + assert_test_outcome "passed", control_id + refute_waiver_annotation control_id + end + + [ + "02_not_waivered_fails", + "07_waivered_expiry_in_past_not_skipped_fails", # Should this give a special waiver expired message? + ].each do |control_id| + assert_test_outcome "failed", control_id + refute_waiver_annotation control_id + end + + # Each of these should have been forced to skip by the waiver system + %w{ + 05_waivered_no_expiry_skipped + 11_waivered_expiry_in_future_skipped + }.each do |control_id| + assert_test_outcome "skipped", control_id + assert_waiver_annotation control_id + end + + # Each of these should have had a failure, but had a waiver annotation + # added to the output. + %w{ + 04_waivered_no_expiry_not_skipped_fails + 10_waivered_expiry_in_future_not_skipped_fails + }.each do |control_id| + assert_test_outcome "failed", control_id + assert_waiver_annotation control_id + end + end + end + + # describe "an inherited profile" + # describe "a profile whose control ids require transformation" + + describe "a waiver file with invalid dates" do + let(:profile_name) { "short" } + let(:waiver_file) { "bad-date.yaml" } + it "gracefully errors" do + result = run_result + assert_includes "ERROR", result.stdout # the error level + assert_includes "01_small", result.stdout # the offending control ID + assert_includes "never", result.stdout # The bad value + assert_equal 1, result.exit_status + end + end + + describe "waivers and only_if" do + let(:profile_name) { "only_if" } + + describe "when an only_if is used with no waiver" do + let(:waiver_file) { "empty.yaml" } + it "skips the control with an only_if message" do + msg = controls_by_id.dig("01_only_if", "results", 0, "skip_message") + assert_includes msg, "due to only_if" + refute_includes msg, "waiver" + end + end + + describe "when both a skipping waiver and an only_if are present" do + let(:waiver_file) { "waiver.yaml" } + it "skips the control with a waiver message" do + msg = controls_by_id.dig("01_only_if", "results", 0, "skip_message") + refute_includes msg, "due to only_if" + assert_includes msg, "waiver" + end + end + end +end diff --git a/test/unit/mock/profiles/waivers/basic/controls/basic.rb b/test/unit/mock/profiles/waivers/basic/controls/basic.rb new file mode 100644 index 000000000..cd3e73045 --- /dev/null +++ b/test/unit/mock/profiles/waivers/basic/controls/basic.rb @@ -0,0 +1,43 @@ +control "01_not_waivered_passes" do + describe(true) { it { should eq true } } +end + +control "02_not_waivered_fails" do + describe(true) { it { should eq false } } +end + +control "03_waivered_no_expiry_not_skipped_passes" do + describe(true) { it { should eq true } } +end + +control "04_waivered_no_expiry_not_skipped_fails" do + describe(true) { it { should eq false } } +end + +control "05_waivered_no_expiry_skipped" do + describe(true) { it { should eq true } } +end + +control "06_waivered_expiry_in_past_not_skipped_passes" do + describe(true) { it { should eq true } } +end + +control "07_waivered_expiry_in_past_not_skipped_fails" do + describe(true) { it { should eq false } } +end + +control "08_waivered_expiry_in_past_skipped" do + describe(true) { it { should eq true } } +end + +control "09_waivered_expiry_in_future_not_skipped_passes" do + describe(true) { it { should eq true } } +end + +control "10_waivered_expiry_in_future_not_skipped_fails" do + describe(true) { it { should eq false } } +end + +control "11_waivered_expiry_in_future_skipped" do + describe(true) { it { should eq true } } +end diff --git a/test/unit/mock/profiles/waivers/basic/files/waivers.yaml b/test/unit/mock/profiles/waivers/basic/files/waivers.yaml new file mode 100644 index 000000000..d422fade4 --- /dev/null +++ b/test/unit/mock/profiles/waivers/basic/files/waivers.yaml @@ -0,0 +1,41 @@ +03_waivered_no_expiry_not_skipped_passes: + justification: Sound reasoning + skip: no + +04_waivered_no_expiry_not_skipped_fails: + justification: Unassailable thinking + skip: no + +05_waivered_no_expiry_skipped: + justification: Sheer cleverness + skip: yes + +06_waivered_expiry_in_past_not_skipped_passes: + expiration_date: 1977-06-01 + justification: Necessity + skip: no + +07_waivered_expiry_in_past_not_skipped_fails: + expiration_date: 1977-06-01 + justification: Whimsy + skip: no + +08_waivered_expiry_in_past_skipped: + expiration_date: 1977-06-01 + justification: Contrariness + skip: yes + +09_waivered_expiry_in_future_not_skipped_passes: + expiration_date: 2077-06-01 + justification: Handwaving + skip: no + +10_waivered_expiry_in_future_not_skipped_fails: + expiration_date: 2077-06-01 + justification: Didn't feel like it + skip: no + +11_waivered_expiry_in_future_skipped: + expiration_date: 2077-06-01 + justification: Lack of imagination + skip: yes diff --git a/test/unit/mock/profiles/waivers/basic/inspec.yml b/test/unit/mock/profiles/waivers/basic/inspec.yml new file mode 100644 index 000000000..40c223ed2 --- /dev/null +++ b/test/unit/mock/profiles/waivers/basic/inspec.yml @@ -0,0 +1,6 @@ +name: basic +license: Apache-2.0 +summary: A profile that demonstrates basic usage of the waiver system +version: 0.1.0 +supports: + platform: os diff --git a/test/unit/mock/profiles/waivers/only_if/controls/only_if_controls.rb b/test/unit/mock/profiles/waivers/only_if/controls/only_if_controls.rb new file mode 100644 index 000000000..003447598 --- /dev/null +++ b/test/unit/mock/profiles/waivers/only_if/controls/only_if_controls.rb @@ -0,0 +1,9 @@ + +# This fixture will cause a "skip due to only if" if waivers are +# not working correctly (should be waivered) +control "01_only_if" do + only_if("test_message_from_dsl") { false } + describe true do + it { should eq true } + end +end diff --git a/test/unit/mock/profiles/waivers/only_if/files/empty.yaml b/test/unit/mock/profiles/waivers/only_if/files/empty.yaml new file mode 100644 index 000000000..e69de29bb diff --git a/test/unit/mock/profiles/waivers/only_if/files/waiver.yaml b/test/unit/mock/profiles/waivers/only_if/files/waiver.yaml new file mode 100644 index 000000000..17ed86972 --- /dev/null +++ b/test/unit/mock/profiles/waivers/only_if/files/waiver.yaml @@ -0,0 +1,3 @@ +01_only_if: + skip: true + justification: test_message_from_waiver diff --git a/test/unit/mock/profiles/waivers/only_if/inspec.yml b/test/unit/mock/profiles/waivers/only_if/inspec.yml new file mode 100644 index 000000000..9208c40af --- /dev/null +++ b/test/unit/mock/profiles/waivers/only_if/inspec.yml @@ -0,0 +1,5 @@ +name: only_if +summary: Verifies waiver/only_if precedence +version: 0.1.0 +supports: + platform: os diff --git a/test/unit/mock/profiles/waivers/small/controls/small.rb b/test/unit/mock/profiles/waivers/small/controls/small.rb new file mode 100644 index 000000000..f5ca650a2 --- /dev/null +++ b/test/unit/mock/profiles/waivers/small/controls/small.rb @@ -0,0 +1,5 @@ +control "01_small" do + describe true do + it { should eq true } + end +end diff --git a/test/unit/mock/profiles/waivers/small/files/bad-date.yaml b/test/unit/mock/profiles/waivers/small/files/bad-date.yaml new file mode 100644 index 000000000..28e327265 --- /dev/null +++ b/test/unit/mock/profiles/waivers/small/files/bad-date.yaml @@ -0,0 +1,4 @@ +01_small: + expiration_date: never + skip: true + justification: Callous disregard diff --git a/test/unit/mock/profiles/waivers/small/inspec.yml b/test/unit/mock/profiles/waivers/small/inspec.yml new file mode 100644 index 000000000..57c2c74fa --- /dev/null +++ b/test/unit/mock/profiles/waivers/small/inspec.yml @@ -0,0 +1,5 @@ +name: small +summary: Test profile for running bad waiver files through InSpec +version: 0.1.0 +supports: + platform: os