-
Notifications
You must be signed in to change notification settings - Fork 112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add unit tests for bundle-audit #133
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
## bundler-audit spec tests | ||
|
||
### Overview of bundler-audit | ||
|
||
[bundler-audit](https://github.com/rubysec/bundler-audit) | ||
|
||
Scans a project's vulnerable versions of gems in the `Gemfile.lock` file and checks for gem sources without TLS. | ||
|
||
The names/versions are compared against the [ruby-advisory-db ](https://github.com/rubysec/ruby-advisory-db). | ||
|
||
To install bundler-audit: | ||
``` | ||
gem install bundler-audit | ||
``` | ||
|
||
The simplest way to run it from the command line is to `cd` to the folder with the `Gemfile.lock` file and call: | ||
``` | ||
bundle-audit check | ||
``` | ||
|
||
In Glue, `bundler-audit` is called with the following argument: | ||
``` | ||
bundle-audit check | ||
``` | ||
|
||
Some other command line options when running bundler-audit locally: | ||
* `--update` - Updates the ruby-advisory-db; | ||
* `--ignore [ADVISORY-ID]` - Ignores specific advisories. | ||
|
||
See [bundler-audit documentation](https://www.rubydoc.info/gems/bundler-audit/frames) for more info. | ||
|
||
### The spec tests | ||
|
||
The specs do not call the bundler-audit tool because this would be too slow (~1 sec per spec test). | ||
Instead the specs rely on stubbing Glue's `runsystem` method (which calls CLI commands). | ||
|
||
In the specs, the return value of `runsystem` is always a canned response. | ||
Either it will be a generic, minimal response, or it will be a snapshot of an actual bundler-audit report. | ||
|
||
The actual reports were generated via the script `generate_reports.sh`. | ||
The targets of the spec tests were set up in a minimal way to produce non-trivial output. | ||
This required a `Gemfile.lock` file per target. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,209 @@ | ||
require 'spec_helper' | ||
require 'rspec/collection_matchers' | ||
require 'glue' | ||
require 'glue/event' | ||
require 'glue/tracker' | ||
require 'glue/tasks' | ||
require 'glue/tasks/bundle-audit' | ||
|
||
describe Glue::BundleAudit do | ||
|
||
BUNDLEAUDIT_TARGETS_PATH = 'spec/tasks/bundle-audit/targets' | ||
|
||
def get_bundleaudit(target = 'nil_target') | ||
trigger = Glue::Event.new(target) | ||
trigger.path = File.join(BUNDLEAUDIT_TARGETS_PATH, target) | ||
tracker = Glue::Tracker.new({}) | ||
Glue::BundleAudit.new(trigger, tracker) | ||
end | ||
|
||
def get_raw_report(target, subtarget = nil) | ||
path = File.join(get_target_path(target, subtarget), "report.txt") | ||
File.read(path).chomp | ||
end | ||
|
||
def get_target_path(target, subtarget = nil) | ||
if subtarget.nil? | ||
File.join(BUNDLEAUDIT_TARGETS_PATH, target) | ||
else | ||
File.join(BUNDLEAUDIT_TARGETS_PATH, target, subtarget) | ||
end | ||
end | ||
|
||
def cli_args(target, subtarget = nil) | ||
[true, "bundle-audit", "check", { chdir: get_target_path(target, subtarget) }] | ||
end | ||
|
||
describe "#initialize" do | ||
let(:task) { @task } | ||
before(:all) { @task = get_bundleaudit } | ||
|
||
it "sets the correct 'name'" do | ||
expect(task.name).to eq('BundleAudit') | ||
end | ||
|
||
it "sets the correct 'stage'" do | ||
expect(task.stage).to eq(:code) | ||
end | ||
|
||
it "sets the correct 'labels'" do | ||
expect(task.labels).to eq(%w[code ruby].to_set) | ||
end | ||
end | ||
|
||
describe '#supported?' do | ||
subject(:task) { get_bundleaudit } | ||
|
||
context "when 'runsystem' cannot run the task" do | ||
before do | ||
allow(Glue).to receive(:notify) # suppress the output | ||
cmd_args = [false, "bundle-audit", "update"] | ||
cmd_str = 'command not found' | ||
allow(task).to receive(:runsystem).with(*cmd_args).and_return(cmd_str) | ||
end | ||
|
||
it { is_expected.not_to be_supported } | ||
|
||
it 'issues a notification' do | ||
expect(Glue).to receive(:notify) | ||
task.supported? | ||
end | ||
end | ||
|
||
context "when 'runsystem' returns an updating message" do | ||
before do | ||
cmd_args = [false, "bundle-audit", "update"] | ||
cmd_str = 'Updating ruby-advisory-db ...' | ||
allow(task).to receive(:runsystem).with(*cmd_args).and_return(cmd_str) | ||
end | ||
|
||
it { is_expected.to be_supported } | ||
end | ||
end | ||
|
||
describe "#run" do | ||
let(:task) { get_bundleaudit target } | ||
let(:minimal_response) { "No vulnerabilities found" } | ||
|
||
before do | ||
allow(Glue).to receive(:notify) # suppress the output | ||
allow(Glue).to receive(:warn) # suppress the output | ||
|
||
# This acts as a guard against actually calling bundle-audit from the CLI. | ||
# (All specs should use canned responses instead.) | ||
allow(task).to receive(:runsystem) do | ||
puts "Warning from rspec -- make sure you're not attempting to call the actual bundle-audit CLI" | ||
puts "within an 'it' block with description '#{self.class.description}'" | ||
minimal_response | ||
end | ||
end | ||
|
||
context "with no Gemfile.lock file in root, and no sub-dirs" do | ||
let(:target) { 'no_findings_no_gemfile_lock' } | ||
|
||
it "does not call bundle-audit on the target" do | ||
expect(task).not_to receive(:runsystem).with(*cli_args(target)) | ||
task.run | ||
end | ||
end | ||
|
||
context 'assuming valid (but minimal) reports' do | ||
context 'with one Gemfile.lock in the root dir' do | ||
let(:target) { 'finding_1' } | ||
|
||
before do | ||
allow(task).to receive(:runsystem).with(*cli_args(target)) | ||
end | ||
|
||
it 'passes the task name to Glue.notify' do | ||
expect(Glue).to receive(:notify).with(/^BundleAudit/) | ||
task.run | ||
end | ||
|
||
it "calls the 'bundle-audit' cli once, from the root dir" do | ||
expect(task).to receive(:runsystem).with(*cli_args(target)) | ||
task.run | ||
end | ||
|
||
end | ||
|
||
end | ||
end | ||
|
||
describe "#analyze" do | ||
let(:task) { get_bundleaudit target } | ||
let(:minimal_response) { "No vulnerabilities found" } | ||
|
||
before do | ||
allow(Glue).to receive(:notify) # suppress the output | ||
allow(Glue).to receive(:warn) # suppress the output | ||
|
||
# This acts as a guard against actually calling bundle-audit from the CLI. | ||
# (All specs should use canned responses instead.) | ||
allow(task).to receive(:runsystem) do | ||
puts "Warning from rspec -- make sure you're not attempting to call the actual bundle-audit API" | ||
puts "within an 'it' block with description '#{self.class.description}'" | ||
minimal_response | ||
end | ||
end | ||
|
||
context "with no Gemfile.lock file in root, and no sub-dirs" do | ||
let(:target) { 'no_findings_no_gemfile_lock' } | ||
subject(:task_findings) { task.findings } | ||
it { is_expected.to eq([]) } | ||
end | ||
|
||
context "with one Gemfile.lock in the root dir" do | ||
let(:raw_report) { get_raw_report(target) } | ||
|
||
before do | ||
allow(task).to receive(:runsystem).with(*cli_args(target)).and_return(raw_report) | ||
task.run | ||
task.analyze | ||
end | ||
|
||
context "with no findings" do | ||
let(:target) { 'no_findings' } | ||
subject(:task_findings) { task.findings } | ||
it { is_expected.to eq([]) } | ||
end | ||
|
||
context "with one finding" do | ||
let(:finding) { task.findings.first } | ||
|
||
context "of unknown criticality" do | ||
let (:target) { 'finding_2_unknown' } | ||
|
||
it "has severity 2" do | ||
expect(finding.severity).to eq(2) | ||
end | ||
end | ||
|
||
context "of low criticality" do | ||
let (:target) { 'finding_1' } | ||
|
||
it "has severity 1" do | ||
expect(finding.severity).to eq(1) | ||
end | ||
end | ||
|
||
context "of medium criticality" do | ||
let (:target) { 'finding_2' } | ||
|
||
it "has severity 2" do | ||
expect(finding.severity).to eq(2) | ||
end | ||
end | ||
|
||
context "of high criticality" do | ||
let (:target) { 'finding_3' } | ||
|
||
it "has severity 3" do | ||
expect(finding.severity).to eq(3) | ||
end | ||
end | ||
end | ||
end | ||
end | ||
|
||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
#!/bin/bash | ||
# Runs bundle-audit on the contents of the 'targets' dir, | ||
# storing the output in 'report.txt' within each target folder. | ||
# Include a 'SKIP.txt' file next to 'package.json' if you don't want snyk to run on that target. | ||
|
||
set -e | ||
|
||
run_bundleaudit_recurs () | ||
{ | ||
if [ -f "Gemfile.lock" ] && [ ! -f "SKIP.txt" ]; then | ||
bundle-audit check > report.txt | ||
fi | ||
|
||
for SUBTARGET in * | ||
do | ||
if [ -d ${SUBTARGET} ]; then | ||
cd ${SUBTARGET} | ||
run_bundleaudit_recurs | ||
cd .. | ||
fi | ||
done | ||
} | ||
|
||
DIR=`dirname $0` | ||
cd "${DIR}/targets/" | ||
run_bundleaudit_recurs |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
GEM | ||
remote: https://rubygems.org/ | ||
specs: | ||
kafo (0.3.1) | ||
|
||
PLATFORMS | ||
ruby | ||
|
||
RUBY VERSION | ||
ruby 2.3.3 | ||
|
||
BUNDLED WITH | ||
1.14.6 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
Name: kafo | ||
Version: 0.3.1 | ||
Advisory: CVE-2014-0135 | ||
Criticality: Low | ||
URL: http://osvdb.org/show/osvdb/106826 | ||
Title: Kafo default_values.yaml Insecure Permissions Local Information Disclosure | ||
Solution: upgrade to ~> 0.3.17, >= 0.5.2 | ||
|
||
Vulnerabilities found! |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
GEM | ||
remote: https://rubygems.org/ | ||
specs: | ||
http (0.7.1) | ||
|
||
PLATFORMS | ||
ruby | ||
|
||
RUBY VERSION | ||
ruby 2.3.3 | ||
|
||
BUNDLED WITH | ||
1.14.6 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
Name: http | ||
Version: 0.7.1 | ||
Advisory: CVE-2015-1828 | ||
Criticality: Medium | ||
URL: https://groups.google.com/forum/#!topic/httprb/jkb4oxwZjkU | ||
Title: HTTPS MitM vulnerability in http.rb | ||
Solution: upgrade to >= 0.7.3, ~> 0.6.4 | ||
|
||
Vulnerabilities found! |
13 changes: 13 additions & 0 deletions
13
spec/tasks/bundle-audit/targets/finding_2_unknown/Gemfile.lock
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
GEM | ||
remote: https://rubygems.org/ | ||
specs: | ||
nokogiri (1.8.2) | ||
|
||
PLATFORMS | ||
ruby | ||
|
||
RUBY VERSION | ||
ruby 2.3.3 | ||
|
||
BUNDLED WITH | ||
1.14.6 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
Name: nokogiri | ||
Version: 1.8.2 | ||
Advisory: CVE-2018-8048 | ||
Criticality: Unknown | ||
URL: https://github.com/sparklemotion/nokogiri/pull/1746 | ||
Title: Revert libxml2 behavior in Nokogiri gem that could cause XSS | ||
Solution: upgrade to >= 1.8.3 | ||
|
||
Vulnerabilities found! |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
GEM | ||
remote: https://rubygems.org/ | ||
specs: | ||
curl | ||
|
||
PLATFORMS | ||
ruby | ||
|
||
RUBY VERSION | ||
ruby 2.3.3 | ||
|
||
BUNDLED WITH | ||
1.14.6 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
Name: curl | ||
Version: | ||
Advisory: CVE-2013-2617 | ||
Criticality: High | ||
URL: http://osvdb.org/show/osvdb/91230 | ||
Title: Curl Gem for Ruby URI Handling Arbitrary Command Injection | ||
Solution: remove or disable this gem until a patch is available! | ||
|
||
Vulnerabilities found! |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
GEM | ||
remote: https://rubygems.org/ | ||
specs: | ||
nokogiri (1.8.4) | ||
|
||
PLATFORMS | ||
ruby | ||
|
||
RUBY VERSION | ||
ruby 2.3.3 | ||
|
||
BUNDLED WITH | ||
1.14.6 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
No vulnerabilities found |
1 change: 1 addition & 0 deletions
1
spec/tasks/bundle-audit/targets/no_findings_no_gemfile_lock/example.txt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Empty |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should have a project discussion about ruby versions ... I think this would effectively also bump the Ruby version ... which in theory is a good thing but just something we should probably stop and think about...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this particular case the ruby version specification is not required since this
Gemfile.lock
is used only as a test vector, it should not have impact on the ruby version being used by the system.This being said, and to make it more explicit, I can remove the references to ruby versions from the test data files referenced in this PR since they don't provide value in this case. What do you think?
P.S. This does not remove the (obviously) very good discussion you propose of bumping the Ruby version but probably that would be in a different scope.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. Thanks. I'm approving this PR and merging.
I think we can probably bump the ruby version easily. I would like to see @omerlh's input too.