Skip to content

Commit

Permalink
Merge pull request #62 from coinbase/WeConnect-patternmatch-additiona…
Browse files Browse the repository at this point in the history
…l-options

Patternmatch config option for including extension
  • Loading branch information
nishils authored Jun 5, 2019
2 parents a95b311 + c65c4d0 commit 18e1cce
Show file tree
Hide file tree
Showing 4 changed files with 136 additions and 10 deletions.
9 changes: 7 additions & 2 deletions docs/scanners/pattern_search.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

This scanner can flag anti-patterns found in a codebase or require that certain strings be present. This might be useful for preventing the use of dangerous methods like `eval()` in Ruby (which might allow for RCE) or `dangerouslySetInnerHTML` in React (which might allow for XSS). By default, all found patterns are added to the info section of the report. If a found pattern is forbidden, this scanner will fail and the `message` will be show to the developer in the report to give additional context on why this was an issue. A `required` pattern must be found in order for the scan to pass.

The scanner also allows options `exclude_extension` and `include_extension` for excluding and including file extensions, respectively. These options can be set globally and per-match. While these options can be combined, exclusions take precedence when extensions conflict (are both included and excluded) in declarations.

The tool [sift](https://sift-tool.org), written in Go, is used to perform the pattern matching.

## Configuration
Expand All @@ -15,8 +17,11 @@ scanner_configs:
forbidden: true
exclude_directory:
- node_modules
exclude_extension:
- md
include_extension:
- js
- erb
- html
- htm
- regex: "# Threat Model"
message: All repos must contain a documented threat model.
required: true
Expand Down
22 changes: 14 additions & 8 deletions lib/salus/scanners/pattern_search.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
# Config file can provide:
# - exclude_directories: Array of directories (GLOB) in the repo to exclude from the search.
# - exclude_extensions: Array of file extensions to exclude from the search.
# The above can also be provided per-match, and will override the global values.
# - include_extensions: Array of file extensions to scan exclusively.
# The above can also be provided per-match, and will override the global values. Exclusions
# take precedence over inclusions if they conflict.
# - matches: Array[Hash]
# regex: <regex> (required) regex to match against.
# forbidden: <boolean> (default false) if true, a hit on this regex will fail the test.
Expand All @@ -16,7 +18,8 @@ module Salus::Scanners
class PatternSearch < Base
def run
global_exclude_directory_flags = flag_list('--exclude-dirs', @config['exclude_directory'])
global_exclude_extension_flags = extension_flag(@config['exclude_extension'])
global_exclude_extension_flags = extension_flag('--exclude-ext', @config['exclude_extension'])
global_include_extension_flags = extension_flag('--ext', @config['include_extension'])

# For each pattern, keep a running history of failures, errors, and hits
# These will be reported on at the end.
Expand All @@ -35,12 +38,15 @@ def run
match_exclude_directory_flags = flag_list(
'--exclude-dirs', match['exclude_directory']
)
match_exclude_extension_flags = extension_flag(match['exclude_extension'])
match_exclude_extension_flags = extension_flag('--exclude-ext', \
match['exclude_extension'])
match_include_extension_flags = extension_flag('--ext', match['include_extension'])

command_string = [
"sift -n -e \"#{match['regex']}\" .",
match_exclude_directory_flags || global_exclude_directory_flags,
match_exclude_extension_flags || global_exclude_extension_flags
match_exclude_extension_flags || global_exclude_extension_flags,
match_include_extension_flags || global_include_extension_flags
].compact.join(' ')
shell_return = run_shell(command_string)

Expand Down Expand Up @@ -108,13 +114,13 @@ def should_run?

private

def extension_flag(file_extensions)
if file_extensions.nil?
def extension_flag(flag, file_extensions)
if file_extensions.nil? || flag.nil?
nil
elsif file_extensions.empty?
elsif file_extensions.empty? || flag.empty?
""
else
flag = '--exclude-ext='
flag << '='
flag << file_extensions.join(',')
end
end
Expand Down
3 changes: 3 additions & 0 deletions spec/fixtures/pattern_search/fancy.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Fancy

This is _fancy_ **formatted**.
112 changes: 112 additions & 0 deletions spec/lib/salus/scanners/pattern_search_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,118 @@
end
end

context 'global inclusions are given' do
it 'should search only through included material' do
repo = Salus::Repo.new('spec/fixtures/pattern_search')

config = {
'matches' => [
{ regex: 'UN' },
{ 'regex' => 'lance', 'forbidden' => true },
{ regex: 'fancy' }
],
'include_extension' => ['md']
}

scanner = Salus::Scanners::PatternSearch.new(repository: repo, config: config)
scanner.run

expect(scanner.report.passed?).to eq(true)
end
end

context 'local inclusions are given' do
it 'should only search through included material' do
repo = Salus::Repo.new('spec/fixtures/pattern_search')

config = {
'matches' => [
{ 'regex' => 'lance', 'forbidden' => true, 'include_extension' => ['md'] }
]
}

scanner = Salus::Scanners::PatternSearch.new(repository: repo, config: config)
scanner.run

expect(scanner.report.passed?).to eq(true)
end

it 'should not search through extensions not explicitly included' do
repo = Salus::Repo.new('spec/fixtures/pattern_search')

config = {
'matches' => [
{ 'regex' => 'UN', 'include_extension' => ['md'] },
{ 'regex' => 'fancy', 'include_extension' => ['md'] }
]
}

scanner = Salus::Scanners::PatternSearch.new(repository: repo, config: config)
scanner.run

expect(scanner.report.passed?).to eq(true)

info = scanner.report.to_h.fetch(:info)
expect(info[:hits].map { |hit| hit[:regex] }).to_not include('UN')
expect(info[:hits].map { |hit| hit[:regex] }).to include('fancy')
end

it 'should coexist with exclusions' do
repo = Salus::Repo.new('spec/fixtures/pattern_search')
config = {
'matches' => [
{ 'regex' => 'fancy', 'include_extension' => ['md'] },
{ 'regex' => 'lance', 'forbidden' => true, 'exclude_extension' => ['txt'], \
'include_extension' => ['md'] }
]
}

scanner = Salus::Scanners::PatternSearch.new(repository: repo, config: config)
scanner.run

expect(scanner.report.passed?).to eq(true)

info = scanner.report.to_h.fetch(:info)
expect(info[:hits].map { |hit| hit[:regex] }).to include('fancy')
end

it 'should handle conflicting local exclusions' do
repo = Salus::Repo.new('spec/fixtures/pattern_search')
config = {
'matches' => [
{ 'regex' => 'fancy', 'include_extension' => ['md'], 'exclude_extension' => ['md'] },
{ 'regex' => 'lance', 'forbidden' => true, 'exclude_extension' => ['txt'], \
'include_extension' => ['md'] }
]
}

scanner = Salus::Scanners::PatternSearch.new(repository: repo, config: config)
scanner.run

expect(scanner.report.passed?).to eq(true)
info = scanner.report.to_h.fetch(:info)
expect(info[:hits].map { |hit| hit[:regex] }).to_not include('fancy')
end

it 'should handle conflicting global exclusions' do
repo = Salus::Repo.new('spec/fixtures/pattern_search')
config = {
'matches' => [
{ 'regex' => 'fancy', 'include_extension' => ['md'] },
{ 'regex' => 'lance', 'forbidden' => true, 'include_extension' => ['md'] }
],
'exclude_extension' => %w[txt md]
}

scanner = Salus::Scanners::PatternSearch.new(repository: repo, config: config)
scanner.run

expect(scanner.report.passed?).to eq(true)
info = scanner.report.to_h.fetch(:info)
expect(info[:hits].map { |hit| hit[:regex] }).to_not include('fancy')
end
end

context 'invalid regex or settings which causes error' do
it 'should record the STDERR of bundle-audit' do
repo = Salus::Repo.new('spec/fixtures/pattern_search')
Expand Down

0 comments on commit 18e1cce

Please sign in to comment.