From d14e3baefa2b05cd24b7524a5226af0e8096cf93 Mon Sep 17 00:00:00 2001 From: Andrea Brancaleoni Date: Tue, 19 Mar 2024 14:46:08 -0300 Subject: [PATCH 1/3] reviewdog: upgrade from grep based to fnmatch based, add per-repo setting --- Gemfile.lock | 1 + assets/cleaner.rb | 48 +++++++++++++++++++++++++++++++--- assets/reviewdog/reviewdog.yml | 5 +--- 3 files changed, 46 insertions(+), 8 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index c945b63d..e64378c9 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -7,6 +7,7 @@ GEM PLATFORMS arm64-darwin-22 + arm64-darwin-23 x86_64-linux DEPENDENCIES diff --git a/assets/cleaner.rb b/assets/cleaner.rb index dd308f13..87c4752e 100755 --- a/assets/cleaner.rb +++ b/assets/cleaner.rb @@ -1,20 +1,60 @@ #!/usr/bin/env ruby require 'optparse' -options = {} +DEFAULT_MATCHER_FILENAME = ".github/security-action-blocklist.txt" + +class Matcher + def initialize(*blocklist_files) + @blocklist = [] + blocklist_files.each do |blf| + next unless File.exist?(blf) + + blocklist = File.read(blf).split("\n").map(&:strip).reject(&:empty?) + # remove empty lines and comments + blocklist.reject! { |r| r.empty? || r.start_with?('#') } + + # remove all matching lines and report + blocklist.reject! do |r| + ret = r =~ /^[*@]+$/ + STDERR.puts "Warning: #{blf} contains a line with only asterisks/at, which will match everything" if ret + ret + end + + @blocklist += blocklist + end + end + + def match?(line) + @blocklist.each do |r| + return true if File.fnmatch?("*#{r}*", line) + end + false + end +end + +options = { + matcher: Matcher.new(DEFAULT_MATCHER_FILENAME) +} OptionParser.new do |opts| opts.banner = "Usage: reviewdog-adapter.rb [options]" opts.on("--svgo", "Add SVGO String") do |v| options[:svgo] = true + options[:matcher] = Matcher.new(DEFAULT_MATCHER_FILENAME, "#{ENV["SCRIPTPATH"]}/dtd/blocklist.txt") end - opts.on("--assignees", "Add SVGO String") do |v| + opts.on("--assignees", "Add Assignees String") do |v| options[:assignees] = true end - opts.on("--sveltegrep", "Remove Extracted Script Extension") do |v| + opts.on("--sveltegrep", "Remove Extracted Script Extension, and use semgrep blocklist") do |v| options[:sveltegrep] = true + options[:matcher] = Matcher.new(DEFAULT_MATCHER_FILENAME, "#{ENV["SCRIPTPATH"]}/semgrep_rules/blocklist.txt") + end + + opts.on("--semgrep", "Use semgrep blocklist") do |v| + options[:semgrep] = true + options[:matcher] = Matcher.new(DEFAULT_MATCHER_FILENAME, "#{ENV["SCRIPTPATH"]}/semgrep_rules/blocklist.txt") end end.parse! @@ -39,5 +79,5 @@ l.gsub!(/$/, "
Cc #{ENV['ASSIGNEES']}") end - puts l + puts l unless options[:matcher].match?(l) end \ No newline at end of file diff --git a/assets/reviewdog/reviewdog.yml b/assets/reviewdog/reviewdog.yml index c4874b55..7b97b073 100644 --- a/assets/reviewdog/reviewdog.yml +++ b/assets/reviewdog/reviewdog.yml @@ -14,8 +14,7 @@ runner: $([ -n "${GITHUB_BASE_REF+set}" ] && echo "--baseline-commit origin/${GITHUB_BASE_REF:-main}") \ --json \ | jq -r '.results[] | "\(.extra.severity[0:1]):\(.path):\(.end.line) \(.extra.message | sub("\n";"
";"g"))

Source: \(.extra.metadata.source)

,\(if .extra.metadata.assignees then .extra.metadata.assignees else "null" end | sub("\n";" ";"g"))"' \ - | grep -f $SCRIPTPATH/semgrep_rules/blocklist.txt -v \ - | $SCRIPTPATH/cleaner.rb --assignees) 2> reviewdog.semgrep.stderr.log + | $SCRIPTPATH/cleaner.rb --semgrep --assignees) 2> reviewdog.semgrep.stderr.log errorformat: - "%t:%f:%l %m" sveltegrep: @@ -46,7 +45,6 @@ runner: '--include=*.extractedscript.html' \ ./ \ | jq -r '.results[] | "\(.extra.severity[0:1]):\(.path):\(.end.line) \(.extra.message | sub("\n";"
";"g"))

Source: \(.extra.metadata.source)

,\(if .extra.metadata.assignees then .extra.metadata.assignees else "null" end | sub("\n";" ";"g"))"' \ - | grep -f $SCRIPTPATH/semgrep_rules/blocklist.txt -v \ | $SCRIPTPATH/cleaner.rb --assignees --sveltegrep && \ find . -type f -name '*.extractedscript.*' -delete) 2> reviewdog.sveltegrep.stderr.log errorformat: @@ -56,7 +54,6 @@ runner: cmd: | set -e (xargs -0 -n1 -a $SCRIPTPATH/all_changed_files.txt $SCRIPTPATH/xmllint.sh \ - | egrep -f $SCRIPTPATH/dtd/blocklist.txt -v \ | $SCRIPTPATH/cleaner.rb --svgo) 2> reviewdog.safesvg.stderr.log errorformat: - "%f:%l: %m" From c656cf2d19175347d1f86310fa20e69c45f5f805 Mon Sep 17 00:00:00 2001 From: Andrea Brancaleoni Date: Wed, 20 Mar 2024 13:42:55 -0300 Subject: [PATCH 2/3] assets/dtd/blocklist.txt: fnmatch expansions --- assets/dtd/blocklist.txt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/assets/dtd/blocklist.txt b/assets/dtd/blocklist.txt index b5d6b6f3..91e2b472 100644 --- a/assets/dtd/blocklist.txt +++ b/assets/dtd/blocklist.txt @@ -1,5 +1,5 @@ -element [^:]+: validity error : ID [^ ]+ already defined -element [^:]+: validity error : No declaration for attribute data-[^ ]+ of element [^:]+ +element *: validity error : ID * already defined +element *: validity error : No declaration for attribute data-* of element * element style: validity error : Element style does not carry attribute type -element svg: validity error : Value for attribute version of svg must be [^ ]+ -third_party\/rust\/[^.]+\.svg +element svg: validity error : Value for attribute version of svg must be * +third_party/rust/**/*.svg From 7207e3554c681301e90c4fa4b4de2432849fc94e Mon Sep 17 00:00:00 2001 From: Andrea Brancaleoni Date: Fri, 22 Mar 2024 12:33:43 -0300 Subject: [PATCH 3/3] cleaner.rb: don't use in-repo configs --- assets/cleaner.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/assets/cleaner.rb b/assets/cleaner.rb index 87c4752e..c33e3894 100755 --- a/assets/cleaner.rb +++ b/assets/cleaner.rb @@ -1,8 +1,6 @@ #!/usr/bin/env ruby require 'optparse' -DEFAULT_MATCHER_FILENAME = ".github/security-action-blocklist.txt" - class Matcher def initialize(*blocklist_files) @blocklist = [] @@ -33,14 +31,14 @@ def match?(line) end options = { - matcher: Matcher.new(DEFAULT_MATCHER_FILENAME) + matcher: Matcher.new() } OptionParser.new do |opts| opts.banner = "Usage: reviewdog-adapter.rb [options]" opts.on("--svgo", "Add SVGO String") do |v| options[:svgo] = true - options[:matcher] = Matcher.new(DEFAULT_MATCHER_FILENAME, "#{ENV["SCRIPTPATH"]}/dtd/blocklist.txt") + options[:matcher] = Matcher.new("#{ENV["SCRIPTPATH"]}/dtd/blocklist.txt") end opts.on("--assignees", "Add Assignees String") do |v| @@ -49,12 +47,12 @@ def match?(line) opts.on("--sveltegrep", "Remove Extracted Script Extension, and use semgrep blocklist") do |v| options[:sveltegrep] = true - options[:matcher] = Matcher.new(DEFAULT_MATCHER_FILENAME, "#{ENV["SCRIPTPATH"]}/semgrep_rules/blocklist.txt") + options[:matcher] = Matcher.new("#{ENV["SCRIPTPATH"]}/semgrep_rules/blocklist.txt") end opts.on("--semgrep", "Use semgrep blocklist") do |v| options[:semgrep] = true - options[:matcher] = Matcher.new(DEFAULT_MATCHER_FILENAME, "#{ENV["SCRIPTPATH"]}/semgrep_rules/blocklist.txt") + options[:matcher] = Matcher.new("#{ENV["SCRIPTPATH"]}/semgrep_rules/blocklist.txt") end end.parse!