From 1736d74408a406468d9f66e4dda1a1492793b8d3 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 11 May 2018 13:28:26 +0200 Subject: [PATCH 01/21] Improve fast specs helper to autoload the library --- spec/fast_spec_helper.rb | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/spec/fast_spec_helper.rb b/spec/fast_spec_helper.rb index 978113a08a4..134eb25e4b1 100644 --- a/spec/fast_spec_helper.rb +++ b/spec/fast_spec_helper.rb @@ -3,14 +3,8 @@ require 'bundler/setup' ENV['GITLAB_ENV'] = 'test' ENV['IN_MEMORY_APPLICATION_SETTINGS'] = 'true' -unless Object.respond_to?(:require_dependency) - class Object - alias_method :require_dependency, :require - end -end - -# Defines Settings and Gitlab.config which are at the center of the app require_relative '../config/settings' -require_relative '../lib/gitlab' unless defined?(Gitlab.config) - require_relative 'support/rspec' +require 'active_support/all' + +ActiveSupport::Dependencies.autoload_paths << 'lib' From 6d0c10b1b791e1938ce42c332280f5fe7dcd489f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 11 May 2018 13:28:51 +0200 Subject: [PATCH 02/21] Make it possible to compare untrusted regexps --- lib/gitlab/untrusted_regexp.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/gitlab/untrusted_regexp.rb b/lib/gitlab/untrusted_regexp.rb index 7ce2e9d636e..fb25755391d 100644 --- a/lib/gitlab/untrusted_regexp.rb +++ b/lib/gitlab/untrusted_regexp.rb @@ -9,7 +9,7 @@ module Gitlab # there is a strict limit on total execution time. See the RE2 documentation # at https://github.com/google/re2/wiki/Syntax for more details. class UntrustedRegexp - delegate :===, to: :regexp + delegate :===, :source, to: :regexp def initialize(pattern) @regexp = RE2::Regexp.new(pattern, log_errors: false) @@ -31,6 +31,10 @@ module Gitlab RE2.Replace(text, regexp, rewrite) end + def ==(other) + self.source == other.source + end + private attr_reader :regexp From 8b736c91fc928157df9ace050f769d0948b58c1d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 11 May 2018 13:29:05 +0200 Subject: [PATCH 03/21] Implement variables expression untrusted pattern lexeme --- .../ci/pipeline/expression/lexeme/pattern.rb | 26 +++++++++ .../expression/lexeme/pattern_spec.rb | 55 +++++++++++++++++++ 2 files changed, 81 insertions(+) create mode 100644 lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb create mode 100644 spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb b/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb new file mode 100644 index 00000000000..8cb1af30252 --- /dev/null +++ b/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb @@ -0,0 +1,26 @@ +module Gitlab + module Ci + module Pipeline + module Expression + module Lexeme + class Pattern < Lexeme::Value + PATTERN = %r{/(?.+)/}.freeze + + def initialize(regexp) + @value = regexp + end + + def evaluate(variables = {}) + Gitlab::UntrustedRegexp.new(@value.to_s) + # TODO raise LexerError / ParserError in case of RegexpError + end + + def self.build(string) + new(string.match(PATTERN)[:regexp]) + end + end + end + end + end + end +end diff --git a/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb new file mode 100644 index 00000000000..b7998b512f5 --- /dev/null +++ b/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb @@ -0,0 +1,55 @@ +require 'fast_spec_helper' +require_dependency 're2' + +describe Gitlab::Ci::Pipeline::Expression::Lexeme::Pattern do + describe '.build' do + it 'creates a new instance of the token' do + expect(described_class.build('/.*/')) + .to be_a(described_class) + end + end + + describe '.type' do + it 'is a value lexeme' do + expect(described_class.type).to eq :value + end + end + + describe '.scan' do + it 'correctly identifies a pattern token' do + scanner = StringScanner.new('/pattern/') + + token = described_class.scan(scanner) + + expect(token).not_to be_nil + expect(token.build.evaluate) + .to eq Gitlab::UntrustedRegexp.new('pattern') + end + + it 'is a greedy scanner for regexp boundaries' do + scanner = StringScanner.new('/some .* / pattern/') + + token = described_class.scan(scanner) + + expect(token).not_to be_nil + expect(token.build.evaluate) + .to eq Gitlab::UntrustedRegexp.new('some .* / pattern') + end + + it 'does not allow to use an empty pattern' do + scanner = StringScanner.new(%(//)) + + token = described_class.scan(scanner) + + expect(token).to be_nil + end + end + + describe '#evaluate' do + it 'returns a regular expression' do + string = described_class.new('abc') + + expect(string.evaluate).to eq Gitlab::UntrustedRegexp.new('abc') + end + end +end From b784a985f2188e328da32cc9fdc73c8d4ac63733 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 14 May 2018 14:24:59 +0200 Subject: [PATCH 04/21] Do not raise if variable expression can not be evaluated --- lib/gitlab/ci/pipeline/expression/statement.rb | 2 ++ .../ci/pipeline/expression/statement_spec.rb | 16 ++++++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/ci/pipeline/expression/statement.rb b/lib/gitlab/ci/pipeline/expression/statement.rb index 09a7c98464b..363e0b708a6 100644 --- a/lib/gitlab/ci/pipeline/expression/statement.rb +++ b/lib/gitlab/ci/pipeline/expression/statement.rb @@ -35,6 +35,8 @@ module Gitlab def truthful? evaluate.present? + rescue StatementError + false end def valid? diff --git a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb index 6685bf5385b..633c932eabb 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb @@ -1,4 +1,4 @@ -require 'spec_helper' +require 'fast_spec_helper' describe Gitlab::Ci::Pipeline::Expression::Statement do subject do @@ -114,7 +114,8 @@ describe Gitlab::Ci::Pipeline::Expression::Statement do ['$UNDEFINED_VARIABLE == null', true], ['$PRESENT_VARIABLE', true], ['$UNDEFINED_VARIABLE', false], - ['$EMPTY_VARIABLE', false] + ['$EMPTY_VARIABLE', false], + ['$INVALID = 1', false] ] statements.each do |expression, value| @@ -126,5 +127,16 @@ describe Gitlab::Ci::Pipeline::Expression::Statement do end end end + + context 'when evaluating expression raises an error' do + let(:text) { '$PRESENT_VARIABLE' } + + it 'returns false' do + allow(subject).to receive(:evaluate) + .and_raise(described_class::StatementError) + + expect(subject.truthful?).to be_falsey + end + end end end From ac65257c40052f739492f0648f6b7c06a1c95250 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 14 May 2018 14:38:08 +0200 Subject: [PATCH 05/21] Raise variables statement exception if pattern is invalid --- lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb | 6 ++++-- lib/gitlab/ci/pipeline/expression/parser.rb | 2 ++ .../ci/pipeline/expression/lexeme/pattern_spec.rb | 11 +++++++++-- spec/lib/gitlab/ci/pipeline/expression/parser_spec.rb | 2 +- spec/lib/gitlab/ci/pipeline/expression/token_spec.rb | 2 +- 5 files changed, 17 insertions(+), 6 deletions(-) diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb b/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb index 8cb1af30252..2ff527e34a8 100644 --- a/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb +++ b/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb @@ -11,8 +11,10 @@ module Gitlab end def evaluate(variables = {}) - Gitlab::UntrustedRegexp.new(@value.to_s) - # TODO raise LexerError / ParserError in case of RegexpError + # TODO multiline support + @regexp = Gitlab::UntrustedRegexp.new(@value) + rescue RegexpError + raise Parser::ParserError, 'Invalid regular expression!' end def self.build(string) diff --git a/lib/gitlab/ci/pipeline/expression/parser.rb b/lib/gitlab/ci/pipeline/expression/parser.rb index 90f94d0b763..fe23ab0b2f8 100644 --- a/lib/gitlab/ci/pipeline/expression/parser.rb +++ b/lib/gitlab/ci/pipeline/expression/parser.rb @@ -3,6 +3,8 @@ module Gitlab module Pipeline module Expression class Parser + ParserError = Class.new(Statement::StatementError) + def initialize(tokens) @tokens = tokens.to_enum @nodes = [] diff --git a/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb index b7998b512f5..ed69742cd7c 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb @@ -47,9 +47,16 @@ describe Gitlab::Ci::Pipeline::Expression::Lexeme::Pattern do describe '#evaluate' do it 'returns a regular expression' do - string = described_class.new('abc') + regexp = described_class.new('abc') - expect(string.evaluate).to eq Gitlab::UntrustedRegexp.new('abc') + expect(regexp.evaluate).to eq Gitlab::UntrustedRegexp.new('abc') + end + + it 'raises error if evaluated regexp is not valid' do + regexp = described_class.new('invalid ( .*') + + expect { regexp.evaluate } + .to raise_error(Gitlab::Ci::Pipeline::Expression::Parser::ParserError) end end end diff --git a/spec/lib/gitlab/ci/pipeline/expression/parser_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/parser_spec.rb index e8e6f585310..2b78b1dd4a7 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/parser_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/parser_spec.rb @@ -1,4 +1,4 @@ -require 'spec_helper' +require 'fast_spec_helper' describe Gitlab::Ci::Pipeline::Expression::Parser do describe '#tree' do diff --git a/spec/lib/gitlab/ci/pipeline/expression/token_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/token_spec.rb index 6d7453f0de5..cedfe270f9d 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/token_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/token_spec.rb @@ -1,4 +1,4 @@ -require 'spec_helper' +require 'fast_spec_helper' describe Gitlab::Ci::Pipeline::Expression::Token do let(:value) { '$VARIABLE' } From f16f2b599412ed1514ba96d81758b9a2e6fd9c1f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 14 May 2018 15:03:10 +0200 Subject: [PATCH 06/21] Add pattern matching variables expression lexeme --- .../ci/pipeline/expression/lexeme/matches.rb | 29 ++++++++++ .../expression/lexeme/matches_spec.rb | 57 +++++++++++++++++++ 2 files changed, 86 insertions(+) create mode 100644 lib/gitlab/ci/pipeline/expression/lexeme/matches.rb create mode 100644 spec/lib/gitlab/ci/pipeline/expression/lexeme/matches_spec.rb diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb b/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb new file mode 100644 index 00000000000..806f2082227 --- /dev/null +++ b/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb @@ -0,0 +1,29 @@ +module Gitlab + module Ci + module Pipeline + module Expression + module Lexeme + class Matches < Lexeme::Operator + PATTERN = /=~/.freeze + + def initialize(left, right) + @left = left + @right = right + end + + def evaluate(variables = {}) + text = @left.evaluate(variables) + regexp = @right.evaluate(variables) + + regexp.scan(text).any? + end + + def self.build(_value, behind, ahead) + new(behind, ahead) + end + end + end + end + end + end +end diff --git a/spec/lib/gitlab/ci/pipeline/expression/lexeme/matches_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/lexeme/matches_spec.rb new file mode 100644 index 00000000000..22907b0554a --- /dev/null +++ b/spec/lib/gitlab/ci/pipeline/expression/lexeme/matches_spec.rb @@ -0,0 +1,57 @@ +require 'fast_spec_helper' +require_dependency 're2' + +describe Gitlab::Ci::Pipeline::Expression::Lexeme::Matches do + let(:left) { double('left') } + let(:right) { double('right') } + + describe '.build' do + it 'creates a new instance of the token' do + expect(described_class.build('=~', left, right)) + .to be_a(described_class) + end + end + + describe '.type' do + it 'is an operator' do + expect(described_class.type).to eq :operator + end + end + + describe '#evaluate' do + it 'returns false when left and right do not match' do + allow(left).to receive(:evaluate).and_return('my-string') + allow(right).to receive(:evaluate) + .and_return(Gitlab::UntrustedRegexp.new('something')) + + operator = described_class.new(left, right) + + expect(operator.evaluate).to eq false + end + + it 'returns true when left and right match' do + allow(left).to receive(:evaluate).and_return('my-awesome-string') + allow(right).to receive(:evaluate) + .and_return(Gitlab::UntrustedRegexp.new('awesome.string$')) + + operator = described_class.new(left, right) + + expect(operator.evaluate).to eq true + end + + it 'supports multiline strings' do + allow(left).to receive(:evaluate).and_return <<~TEXT + My awesome contents + + My-text-string! + TEXT + + allow(right).to receive(:evaluate) + .and_return(Gitlab::UntrustedRegexp.new('text-string')) + + operator = described_class.new(left, right) + + expect(operator.evaluate).to eq true + end + end +end From 475d2edf04c13c6a5ed2b5c68d75efd2a8024c2b Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 15 May 2018 12:37:09 +0200 Subject: [PATCH 07/21] Reorganize exceptions in pipeline expressions module --- lib/gitlab/ci/pipeline/expression.rb | 10 ++++++++++ lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb | 3 +-- lib/gitlab/ci/pipeline/expression/lexer.rb | 4 ++-- lib/gitlab/ci/pipeline/expression/parser.rb | 2 -- lib/gitlab/ci/pipeline/expression/statement.rb | 6 +++--- .../ci/pipeline/expression/lexeme/pattern_spec.rb | 2 +- spec/lib/gitlab/ci/pipeline/expression/lexer_spec.rb | 8 ++++---- .../gitlab/ci/pipeline/expression/statement_spec.rb | 4 ++-- 8 files changed, 23 insertions(+), 16 deletions(-) create mode 100644 lib/gitlab/ci/pipeline/expression.rb diff --git a/lib/gitlab/ci/pipeline/expression.rb b/lib/gitlab/ci/pipeline/expression.rb new file mode 100644 index 00000000000..f57df7c5637 --- /dev/null +++ b/lib/gitlab/ci/pipeline/expression.rb @@ -0,0 +1,10 @@ +module Gitlab + module Ci + module Pipeline + module Expression + ExpressionError = Class.new(StandardError) + RuntimeError = Class.new(ExpressionError) + end + end + end +end diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb b/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb index 2ff527e34a8..59b8e4fad4c 100644 --- a/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb +++ b/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb @@ -11,10 +11,9 @@ module Gitlab end def evaluate(variables = {}) - # TODO multiline support @regexp = Gitlab::UntrustedRegexp.new(@value) rescue RegexpError - raise Parser::ParserError, 'Invalid regular expression!' + raise Expression::RuntimeError, 'Invalid regular expression!' end def self.build(string) diff --git a/lib/gitlab/ci/pipeline/expression/lexer.rb b/lib/gitlab/ci/pipeline/expression/lexer.rb index e1c68b7c3c2..ebc6565266f 100644 --- a/lib/gitlab/ci/pipeline/expression/lexer.rb +++ b/lib/gitlab/ci/pipeline/expression/lexer.rb @@ -5,6 +5,8 @@ module Gitlab class Lexer include ::Gitlab::Utils::StrongMemoize + SyntaxError = Class.new(Expression::ExpressionError) + LEXEMES = [ Expression::Lexeme::Variable, Expression::Lexeme::String, @@ -12,8 +14,6 @@ module Gitlab Expression::Lexeme::Equals ].freeze - SyntaxError = Class.new(Statement::StatementError) - MAX_TOKENS = 100 def initialize(statement, max_tokens: MAX_TOKENS) diff --git a/lib/gitlab/ci/pipeline/expression/parser.rb b/lib/gitlab/ci/pipeline/expression/parser.rb index fe23ab0b2f8..90f94d0b763 100644 --- a/lib/gitlab/ci/pipeline/expression/parser.rb +++ b/lib/gitlab/ci/pipeline/expression/parser.rb @@ -3,8 +3,6 @@ module Gitlab module Pipeline module Expression class Parser - ParserError = Class.new(Statement::StatementError) - def initialize(tokens) @tokens = tokens.to_enum @nodes = [] diff --git a/lib/gitlab/ci/pipeline/expression/statement.rb b/lib/gitlab/ci/pipeline/expression/statement.rb index 363e0b708a6..de37c50d2a2 100644 --- a/lib/gitlab/ci/pipeline/expression/statement.rb +++ b/lib/gitlab/ci/pipeline/expression/statement.rb @@ -3,7 +3,7 @@ module Gitlab module Pipeline module Expression class Statement - StatementError = Class.new(StandardError) + StatementError = Class.new(Expression::ExpressionError) GRAMMAR = [ %w[variable equals string], @@ -35,13 +35,13 @@ module Gitlab def truthful? evaluate.present? - rescue StatementError + rescue Expression::ExpressionError false end def valid? parse_tree.is_a?(Lexeme::Base) - rescue StatementError + rescue Expression::ExpressionError false end end diff --git a/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb index ed69742cd7c..47385ce0a5b 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb @@ -56,7 +56,7 @@ describe Gitlab::Ci::Pipeline::Expression::Lexeme::Pattern do regexp = described_class.new('invalid ( .*') expect { regexp.evaluate } - .to raise_error(Gitlab::Ci::Pipeline::Expression::Parser::ParserError) + .to raise_error(Gitlab::Ci::Pipeline::Expression::RuntimeError) end end end diff --git a/spec/lib/gitlab/ci/pipeline/expression/lexer_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/lexer_spec.rb index 230ceeb07f8..3f11b3f7673 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/lexer_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/lexer_spec.rb @@ -6,7 +6,7 @@ describe Gitlab::Ci::Pipeline::Expression::Lexer do end describe '#tokens' do - it 'tokenss single value' do + it 'returns single value' do tokens = described_class.new('$VARIABLE').tokens expect(tokens).to be_one @@ -20,14 +20,14 @@ describe Gitlab::Ci::Pipeline::Expression::Lexer do expect(tokens).to all(be_an_instance_of(token_class)) end - it 'tokenss multiple values of the same token' do + it 'returns multiple values of the same token' do tokens = described_class.new("$VARIABLE1 $VARIABLE2").tokens expect(tokens.size).to eq 2 expect(tokens).to all(be_an_instance_of(token_class)) end - it 'tokenss multiple values with different tokens' do + it 'returns multiple values with different tokens' do tokens = described_class.new('$VARIABLE "text" "value"').tokens expect(tokens.size).to eq 3 @@ -36,7 +36,7 @@ describe Gitlab::Ci::Pipeline::Expression::Lexer do expect(tokens.third.value).to eq '"value"' end - it 'tokenss tokens and operators' do + it 'returns tokens and operators' do tokens = described_class.new('$VARIABLE == "text"').tokens expect(tokens.size).to eq 3 diff --git a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb index 633c932eabb..6d58838bf14 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb @@ -36,7 +36,7 @@ describe Gitlab::Ci::Pipeline::Expression::Statement do '== "123"', # invalid left side '"some string"', # only string provided '$VAR ==', # invalid right side - '12345', # unknown syntax + 'null', # missing lexemes '' # empty statement ] @@ -44,7 +44,7 @@ describe Gitlab::Ci::Pipeline::Expression::Statement do context "when expression grammar is #{syntax.inspect}" do let(:text) { syntax } - it 'aises a statement error exception' do + it 'raises a statement error exception' do expect { subject.parse_tree } .to raise_error described_class::StatementError end From 65f4e7b2a1fe8946109c7aa0d59999bfaaeba257 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 15 May 2018 13:04:18 +0200 Subject: [PATCH 08/21] Add support for pattern matching in variables expressions --- lib/gitlab/ci/pipeline/expression/lexeme/matches.rb | 2 +- lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb | 2 ++ lib/gitlab/ci/pipeline/expression/lexer.rb | 4 +++- lib/gitlab/ci/pipeline/expression/statement.rb | 4 +++- .../ci/pipeline/expression/lexeme/matches_spec.rb | 10 ++++++++++ .../ci/pipeline/expression/lexeme/pattern_spec.rb | 1 - .../gitlab/ci/pipeline/expression/statement_spec.rb | 11 ++++++++--- 7 files changed, 27 insertions(+), 7 deletions(-) diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb b/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb index 806f2082227..10957598f76 100644 --- a/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb +++ b/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb @@ -15,7 +15,7 @@ module Gitlab text = @left.evaluate(variables) regexp = @right.evaluate(variables) - regexp.scan(text).any? + regexp.scan(text.to_s).any? end def self.build(_value, behind, ahead) diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb b/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb index 59b8e4fad4c..f7b12914249 100644 --- a/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb +++ b/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb @@ -3,6 +3,8 @@ module Gitlab module Pipeline module Expression module Lexeme + require_dependency 're2' + class Pattern < Lexeme::Value PATTERN = %r{/(?.+)/}.freeze diff --git a/lib/gitlab/ci/pipeline/expression/lexer.rb b/lib/gitlab/ci/pipeline/expression/lexer.rb index ebc6565266f..4cacb1e62c9 100644 --- a/lib/gitlab/ci/pipeline/expression/lexer.rb +++ b/lib/gitlab/ci/pipeline/expression/lexer.rb @@ -10,8 +10,10 @@ module Gitlab LEXEMES = [ Expression::Lexeme::Variable, Expression::Lexeme::String, + Expression::Lexeme::Pattern, Expression::Lexeme::Null, - Expression::Lexeme::Equals + Expression::Lexeme::Equals, + Expression::Lexeme::Matches ].freeze MAX_TOKENS = 100 diff --git a/lib/gitlab/ci/pipeline/expression/statement.rb b/lib/gitlab/ci/pipeline/expression/statement.rb index de37c50d2a2..8886cbae516 100644 --- a/lib/gitlab/ci/pipeline/expression/statement.rb +++ b/lib/gitlab/ci/pipeline/expression/statement.rb @@ -6,12 +6,14 @@ module Gitlab StatementError = Class.new(Expression::ExpressionError) GRAMMAR = [ + %w[variable], %w[variable equals string], %w[variable equals variable], %w[variable equals null], %w[string equals variable], %w[null equals variable], - %w[variable] + %w[variable matches pattern], + %w[pattern matches variable] ].freeze def initialize(statement, variables = {}) diff --git a/spec/lib/gitlab/ci/pipeline/expression/lexeme/matches_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/lexeme/matches_spec.rb index 22907b0554a..a8890262402 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/lexeme/matches_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/lexeme/matches_spec.rb @@ -39,6 +39,16 @@ describe Gitlab::Ci::Pipeline::Expression::Lexeme::Matches do expect(operator.evaluate).to eq true end + it 'supports matching against a nil value' do + allow(left).to receive(:evaluate).and_return(nil) + allow(right).to receive(:evaluate) + .and_return(Gitlab::UntrustedRegexp.new('pattern')) + + operator = described_class.new(left, right) + + expect(operator.evaluate).to eq false + end + it 'supports multiline strings' do allow(left).to receive(:evaluate).and_return <<~TEXT My awesome contents diff --git a/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb index 47385ce0a5b..a14a28056d8 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb @@ -1,5 +1,4 @@ require 'fast_spec_helper' -require_dependency 're2' describe Gitlab::Ci::Pipeline::Expression::Lexeme::Pattern do describe '.build' do diff --git a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb index 6d58838bf14..a5733c13768 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb @@ -84,7 +84,6 @@ describe Gitlab::Ci::Pipeline::Expression::Statement do describe '#evaluate' do statements = [ ['$PRESENT_VARIABLE == "my variable"', true], - ["$PRESENT_VARIABLE == 'my variable'", true], ['"my variable" == $PRESENT_VARIABLE', true], ['$PRESENT_VARIABLE == null', false], ['$EMPTY_VARIABLE == null', false], @@ -93,7 +92,11 @@ describe Gitlab::Ci::Pipeline::Expression::Statement do ['$UNDEFINED_VARIABLE == null', true], ['null == $UNDEFINED_VARIABLE', true], ['$PRESENT_VARIABLE', 'my variable'], - ['$UNDEFINED_VARIABLE', nil] + ['$UNDEFINED_VARIABLE', nil], + ["$PRESENT_VARIABLE =~ /var.*e$/", true], + ["$PRESENT_VARIABLE =~ /^var.*/", false], + ["$EMPTY_VARIABLE =~ /var.*/", false], + ["$UNDEFINED_VARIABLE =~ /var.*/", false] ] statements.each do |expression, value| @@ -115,7 +118,9 @@ describe Gitlab::Ci::Pipeline::Expression::Statement do ['$PRESENT_VARIABLE', true], ['$UNDEFINED_VARIABLE', false], ['$EMPTY_VARIABLE', false], - ['$INVALID = 1', false] + ['$INVALID = 1', false], + ["$PRESENT_VARIABLE =~ /var.*/", true], + ["$UNDEFINED_VARIABLE =~ /var.*/", false] ] statements.each do |expression, value| From 7babc59e4713e53162ac028eb570b78988d3bd6c Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 15 May 2018 13:15:38 +0200 Subject: [PATCH 09/21] Use parameterized RSpec to improve variables expressions specs --- .../ci/pipeline/expression/statement_spec.rb | 79 ++++++++++--------- 1 file changed, 40 insertions(+), 39 deletions(-) diff --git a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb index a5733c13768..bba5db7904a 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb @@ -1,4 +1,5 @@ require 'fast_spec_helper' +require 'rspec-parameterized' describe Gitlab::Ci::Pipeline::Expression::Statement do subject do @@ -82,54 +83,54 @@ describe Gitlab::Ci::Pipeline::Expression::Statement do end describe '#evaluate' do - statements = [ - ['$PRESENT_VARIABLE == "my variable"', true], - ['"my variable" == $PRESENT_VARIABLE', true], - ['$PRESENT_VARIABLE == null', false], - ['$EMPTY_VARIABLE == null', false], - ['"" == $EMPTY_VARIABLE', true], - ['$EMPTY_VARIABLE', ''], - ['$UNDEFINED_VARIABLE == null', true], - ['null == $UNDEFINED_VARIABLE', true], - ['$PRESENT_VARIABLE', 'my variable'], - ['$UNDEFINED_VARIABLE', nil], - ["$PRESENT_VARIABLE =~ /var.*e$/", true], - ["$PRESENT_VARIABLE =~ /^var.*/", false], - ["$EMPTY_VARIABLE =~ /var.*/", false], - ["$UNDEFINED_VARIABLE =~ /var.*/", false] - ] + using RSpec::Parameterized::TableSyntax - statements.each do |expression, value| - context "when using expression `#{expression}`" do - let(:text) { expression } + where(:expression, :value) do + '$PRESENT_VARIABLE == "my variable"' | true + '"my variable" == $PRESENT_VARIABLE' | true + '$PRESENT_VARIABLE == null' | false + '$EMPTY_VARIABLE == null' | false + '"" == $EMPTY_VARIABLE' | true + '$EMPTY_VARIABLE' | '' + '$UNDEFINED_VARIABLE == null' | true + 'null == $UNDEFINED_VARIABLE' | true + '$PRESENT_VARIABLE' | 'my variable' + '$UNDEFINED_VARIABLE' | nil + "$PRESENT_VARIABLE =~ /var.*e$/" | true + "$PRESENT_VARIABLE =~ /^var.*/" | false + "$EMPTY_VARIABLE =~ /var.*/" | false + "$UNDEFINED_VARIABLE =~ /var.*/" | false + end - it "evaluates to `#{value.inspect}`" do - expect(subject.evaluate).to eq value - end + with_them do + let(:text) { expression } + + it "evaluates to `#{params[:value].inspect}`" do + expect(subject.evaluate).to eq value end end end describe '#truthful?' do - statements = [ - ['$PRESENT_VARIABLE == "my variable"', true], - ["$PRESENT_VARIABLE == 'no match'", false], - ['$UNDEFINED_VARIABLE == null', true], - ['$PRESENT_VARIABLE', true], - ['$UNDEFINED_VARIABLE', false], - ['$EMPTY_VARIABLE', false], - ['$INVALID = 1', false], - ["$PRESENT_VARIABLE =~ /var.*/", true], - ["$UNDEFINED_VARIABLE =~ /var.*/", false] - ] + using RSpec::Parameterized::TableSyntax - statements.each do |expression, value| - context "when using expression `#{expression}`" do - let(:text) { expression } + where(:expression, :value) do + '$PRESENT_VARIABLE == "my variable"' | true + "$PRESENT_VARIABLE == 'no match'" | false + '$UNDEFINED_VARIABLE == null' | true + '$PRESENT_VARIABLE' | true + '$UNDEFINED_VARIABLE' | false + '$EMPTY_VARIABLE' | false + '$INVALID = 1' | false + "$PRESENT_VARIABLE =~ /var.*/" | true + "$UNDEFINED_VARIABLE =~ /var.*/" | false + end - it "returns `#{value.inspect}`" do - expect(subject.truthful?).to eq value - end + with_them do + let(:text) { expression } + + it "returns `#{params[:value].inspect}`" do + expect(subject.truthful?).to eq value end end From df91580cc400897c5bf69d5e496b2e4db2f75e4f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 15 May 2018 13:31:45 +0200 Subject: [PATCH 10/21] Do not support inverse variable pattern matching --- lib/gitlab/ci/pipeline/expression/statement.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/gitlab/ci/pipeline/expression/statement.rb b/lib/gitlab/ci/pipeline/expression/statement.rb index 8886cbae516..b36f1e0f865 100644 --- a/lib/gitlab/ci/pipeline/expression/statement.rb +++ b/lib/gitlab/ci/pipeline/expression/statement.rb @@ -12,8 +12,7 @@ module Gitlab %w[variable equals null], %w[string equals variable], %w[null equals variable], - %w[variable matches pattern], - %w[pattern matches variable] + %w[variable matches pattern] ].freeze def initialize(statement, variables = {}) From 9b213583937d73c79115a043b7733c76f79f5d3c Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 15 May 2018 13:40:17 +0200 Subject: [PATCH 11/21] Add variables expressions regexp support changelog --- .../feature-gb-add-regexp-variables-expression.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/feature-gb-add-regexp-variables-expression.yml diff --git a/changelogs/unreleased/feature-gb-add-regexp-variables-expression.yml b/changelogs/unreleased/feature-gb-add-regexp-variables-expression.yml new file mode 100644 index 00000000000..d77c5b42497 --- /dev/null +++ b/changelogs/unreleased/feature-gb-add-regexp-variables-expression.yml @@ -0,0 +1,5 @@ +--- +title: Add support for variables expression pattern matching syntax +merge_request: 18902 +author: +type: added From c1377c6cf0647cccd04bc6fb65d745a2e446f827 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 15 May 2018 14:35:12 +0200 Subject: [PATCH 12/21] Remove useless assignment in pattern lexeme --- lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb b/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb index f7b12914249..62927441035 100644 --- a/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb +++ b/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb @@ -13,7 +13,7 @@ module Gitlab end def evaluate(variables = {}) - @regexp = Gitlab::UntrustedRegexp.new(@value) + Gitlab::UntrustedRegexp.new(@value) rescue RegexpError raise Expression::RuntimeError, 'Invalid regular expression!' end From 73aee958e2f4f7428a961c3c63323a7b782599bf Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 15 May 2018 14:41:20 +0200 Subject: [PATCH 13/21] Add docs on pattern matching syntax in variables expression --- doc/ci/variables/README.md | 7 +++++++ doc/ci/yaml/README.md | 13 ++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md index 42367bf13f7..cbd2ab979f4 100644 --- a/doc/ci/variables/README.md +++ b/doc/ci/variables/README.md @@ -530,6 +530,13 @@ Below you can find supported syntax reference: `$STAGING` value needs to a string, with length higher than zero. Variable that contains only whitespace characters is not an empty variable. +1. Pattern matching _(added in 11.0)_ + + > Example: `$VARIABLE =~ /^content.*/` + + It is possible perform pattern matching against a variable and regular + expression. Expression like this evaluates to truth if matches are found. + ### Unsupported predefined variables Because GitLab evaluates variables before creating jobs, we do not support a diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 2a17a51d7f8..3e77a6f58b7 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -344,10 +344,11 @@ job: kubernetes: active ``` -Example of using variables expressions: +Examples of using variables expressions: ```yaml deploy: + script: cap staging deploy only: refs: - branches @@ -356,6 +357,16 @@ deploy: - $STAGING ``` +Another use case is exluding jobs depending on a commit message _(added in 11.0)_: + +```yaml +end-to-end: + script: rake test:end-to-end + except: + variables: + - $CI_COMMIT_MESSAGE =~ /skip-end-to-end-tests/ +``` + Learn more about variables expressions on [a separate page][variables-expressions]. ## `tags` From f52de2f73cc9d26c26fd66c23892ac42bf973b05 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 15 May 2018 15:18:18 +0200 Subject: [PATCH 14/21] Make variables expression pattern case-sensitivity explicit --- doc/ci/variables/README.md | 8 ++++++++ .../ci/pipeline/expression/lexeme/matches_spec.rb | 13 +++++++++++++ .../gitlab/ci/pipeline/expression/statement_spec.rb | 1 + 3 files changed, 22 insertions(+) diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md index cbd2ab979f4..58acc030aee 100644 --- a/doc/ci/variables/README.md +++ b/doc/ci/variables/README.md @@ -537,6 +537,12 @@ Below you can find supported syntax reference: It is possible perform pattern matching against a variable and regular expression. Expression like this evaluates to truth if matches are found. + Pattern matching is case-sensitive by default. Prepend `(?i)` to a + match-group to make a pattern case-insensitive. + + Under the hood we are using [RE2 library][re2-library], see + [syntax documentation][re2-syntax] for reference. + ### Unsupported predefined variables Because GitLab evaluates variables before creating jobs, we do not support a @@ -577,3 +583,5 @@ These variables are also not supported in a context of a [builds-policies]: ../yaml/README.md#only-and-except-complex [dynamic-environments]: ../environments.md#dynamic-environments [gitlab-deploy-token]: ../../user/project/deploy_tokens/index.md#gitlab-deploy-token +[re2-library]: https://github.com/google/re2 +[re2-syntax]: https://github.com/google/re2/wiki/Syntax diff --git a/spec/lib/gitlab/ci/pipeline/expression/lexeme/matches_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/lexeme/matches_spec.rb index a8890262402..49e5af52f4d 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/lexeme/matches_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/lexeme/matches_spec.rb @@ -63,5 +63,18 @@ describe Gitlab::Ci::Pipeline::Expression::Lexeme::Matches do expect(operator.evaluate).to eq true end + + it 'supports regexp flags' do + allow(left).to receive(:evaluate).and_return <<~TEXT + My AWESOME content + TEXT + + allow(right).to receive(:evaluate) + .and_return(Gitlab::UntrustedRegexp.new('(?i)awesome')) + + operator = described_class.new(left, right) + + expect(operator.evaluate).to eq true + end end end diff --git a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb index bba5db7904a..1ceb373e19c 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb @@ -100,6 +100,7 @@ describe Gitlab::Ci::Pipeline::Expression::Statement do "$PRESENT_VARIABLE =~ /^var.*/" | false "$EMPTY_VARIABLE =~ /var.*/" | false "$UNDEFINED_VARIABLE =~ /var.*/" | false + "$PRESENT_VARIABLE =~ /(?i)VAR.*/" | true end with_them do From 0ce63efe966840edb6e6184cf1abcef272a24dfc Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 17 May 2018 12:29:47 +0200 Subject: [PATCH 15/21] Add extended /regexp/ scheme support to untrusted regexp --- lib/gitlab/untrusted_regexp.rb | 23 +++++++++++ spec/lib/gitlab/untrusted_regexp_spec.rb | 49 +++++++++++++++++++++++- 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/lib/gitlab/untrusted_regexp.rb b/lib/gitlab/untrusted_regexp.rb index ce1cf737663..70d1a7c6535 100644 --- a/lib/gitlab/untrusted_regexp.rb +++ b/lib/gitlab/untrusted_regexp.rb @@ -9,6 +9,8 @@ module Gitlab # there is a strict limit on total execution time. See the RE2 documentation # at https://github.com/google/re2/wiki/Syntax for more details. class UntrustedRegexp + require_dependency 're2' + delegate :===, :source, to: :regexp def initialize(pattern, multiline: false) @@ -52,6 +54,27 @@ module Gitlab Regexp.new(pattern) end + def self.valid?(pattern) + self.fabricate(pattern) + rescue RegexpError + false + end + + def self.fabricate(pattern) + matches = pattern.match(%r{^/(?.+)/(?[ismU]*)$}) + + if matches + expression = matches[:regexp] + flags = matches[:flags] + + expression.prepend("(?#{flags})") if flags.present? + + self.new(expression, multiline: false) + else + self.new(pattern, multiline: false) + end + end + private attr_reader :regexp diff --git a/spec/lib/gitlab/untrusted_regexp_spec.rb b/spec/lib/gitlab/untrusted_regexp_spec.rb index 0ee7fa1e570..4bca320ac2c 100644 --- a/spec/lib/gitlab/untrusted_regexp_spec.rb +++ b/spec/lib/gitlab/untrusted_regexp_spec.rb @@ -1,6 +1,53 @@ -require 'spec_helper' +require 'fast_spec_helper' +require 'support/shared_examples/malicious_regexp_shared_examples' describe Gitlab::UntrustedRegexp do + describe '.valid?' do + it 'returns true if regexp is valid' do + end + + it 'returns true if regexp is invalid' do + end + end + + describe '.fabricate' do + context 'when regexp is using /regexp/ scheme with flags' do + it 'fabricates regexp with a single flag' do + regexp = described_class.fabricate('/something/i') + + expect(regexp).to eq described_class.new('(?i)something') + expect(regexp.scan('SOMETHING')).to be_one + end + + it 'fabricates regexp with multiple flags' do + regexp = described_class.fabricate('/something/im') + + expect(regexp).to eq described_class.new('(?im)something') + end + + it 'fabricates regexp without flags' do + regexp = described_class.fabricate('/something/') + + expect(regexp).to eq described_class.new('something') + end + end + + context 'when regexp is not plain pattern' do + it 'fabricates regexp without flags' do + regexp = described_class.fabricate('something') + + expect(regexp).to eq described_class.new('something') + end + end + + context 'when regexp is invalid' do + it 'raises an error' do + expect { described_class.fabricate('/some ( thing/') } + .to raise_error(RegexpError) + end + end + end + describe '#initialize' do subject { described_class.new(pattern) } From a1f1e08670a7f8bd5499e16c778be16106210a44 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 17 May 2018 12:35:20 +0200 Subject: [PATCH 16/21] Add anti-corruption layer above expressions pattern matching --- .../ci/pipeline/expression/lexeme/pattern.rb | 4 +-- .../expression/lexeme/pattern_spec.rb | 28 +++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb b/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb index 62927441035..53fb5f769d8 100644 --- a/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb +++ b/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb @@ -6,14 +6,14 @@ module Gitlab require_dependency 're2' class Pattern < Lexeme::Value - PATTERN = %r{/(?.+)/}.freeze + PATTERN = %r{^(?/.+/[ismU]*)$}.freeze def initialize(regexp) @value = regexp end def evaluate(variables = {}) - Gitlab::UntrustedRegexp.new(@value) + Gitlab::UntrustedRegexp.fabricate(@value) rescue RegexpError raise Expression::RuntimeError, 'Invalid regular expression!' end diff --git a/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb index a14a28056d8..6435ee5c915 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb @@ -42,6 +42,34 @@ describe Gitlab::Ci::Pipeline::Expression::Lexeme::Pattern do expect(token).to be_nil end + + it 'support single flag' do + scanner = StringScanner.new('/pattern/i') + + token = described_class.scan(scanner) + + expect(token).not_to be_nil + expect(token.build.evaluate) + .to eq Gitlab::UntrustedRegexp.new('(?i)pattern') + end + + it 'support multiple flags' do + scanner = StringScanner.new('/pattern/im') + + token = described_class.scan(scanner) + + expect(token).not_to be_nil + expect(token.build.evaluate) + .to eq Gitlab::UntrustedRegexp.new('(?im)pattern') + end + + it 'does not support arbitrary flags' do + scanner = StringScanner.new('/pattern/x') + + token = described_class.scan(scanner) + + expect(token).to be_nil + end end describe '#evaluate' do From 8b3e21b66b734b38e88f63727ee77b978ea21bfc Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 17 May 2018 12:44:46 +0200 Subject: [PATCH 17/21] Add variables expression pattern validation support --- lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb | 4 ++++ spec/lib/gitlab/ci/config/entry/policy_spec.rb | 10 +++++++++- .../ci/pipeline/expression/lexeme/pattern_spec.rb | 7 +++++++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb b/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb index 53fb5f769d8..70a221010f3 100644 --- a/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb +++ b/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb @@ -10,6 +10,10 @@ module Gitlab def initialize(regexp) @value = regexp + + unless Gitlab::UntrustedRegexp.valid?(@value) + raise Lexer::SyntaxError, 'Invalid regular expression!' + end end def evaluate(variables = {}) diff --git a/spec/lib/gitlab/ci/config/entry/policy_spec.rb b/spec/lib/gitlab/ci/config/entry/policy_spec.rb index 08718c382b9..83d39b82068 100644 --- a/spec/lib/gitlab/ci/config/entry/policy_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/policy_spec.rb @@ -111,7 +111,15 @@ describe Gitlab::Ci::Config::Entry::Policy do context 'when specifying invalid variables expressions token' do let(:config) { { variables: ['$MY_VAR == 123'] } } - it 'reports an error about invalid statement' do + it 'reports an error about invalid expression' do + expect(entry.errors).to include /invalid expression syntax/ + end + end + + context 'when using invalid variables expressions regexp' do + let(:config) { { variables: ['$MY_VAR =~ /some ( thing/'] } } + + it 'reports an error about invalid expression' do expect(entry.errors).to include /invalid expression syntax/ end end diff --git a/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb index 6435ee5c915..c63c38b1dbc 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb @@ -6,6 +6,11 @@ describe Gitlab::Ci::Pipeline::Expression::Lexeme::Pattern do expect(described_class.build('/.*/')) .to be_a(described_class) end + + it 'raises an error if pattern is invalid' do + expect { described_class.build('/ some ( thin/i') } + .to raise_error(Gitlab::Ci::Pipeline::Expression::Lexer::SyntaxError) + end end describe '.type' do @@ -80,6 +85,8 @@ describe Gitlab::Ci::Pipeline::Expression::Lexeme::Pattern do end it 'raises error if evaluated regexp is not valid' do + allow(Gitlab::UntrustedRegexp).to receive(:valid?).and_return(true) + regexp = described_class.new('invalid ( .*') expect { regexp.evaluate } From c808de25515f9c977270a0663796ce34d1c81fd1 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 17 May 2018 12:47:41 +0200 Subject: [PATCH 18/21] Update variables expressions pattern matching docs --- doc/ci/variables/README.md | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md index 58acc030aee..f66b2b374ba 100644 --- a/doc/ci/variables/README.md +++ b/doc/ci/variables/README.md @@ -537,11 +537,8 @@ Below you can find supported syntax reference: It is possible perform pattern matching against a variable and regular expression. Expression like this evaluates to truth if matches are found. - Pattern matching is case-sensitive by default. Prepend `(?i)` to a - match-group to make a pattern case-insensitive. - - Under the hood we are using [RE2 library][re2-library], see - [syntax documentation][re2-syntax] for reference. + Pattern matching is case-sensitive by default. Use `i` flag modifier, like + `/pattern/i` to make a pattern case-insensitive. ### Unsupported predefined variables @@ -583,5 +580,3 @@ These variables are also not supported in a context of a [builds-policies]: ../yaml/README.md#only-and-except-complex [dynamic-environments]: ../environments.md#dynamic-environments [gitlab-deploy-token]: ../../user/project/deploy_tokens/index.md#gitlab-deploy-token -[re2-library]: https://github.com/google/re2 -[re2-syntax]: https://github.com/google/re2/wiki/Syntax From 61d55b56ab4ee7d11484eeea6fabb2412af6578f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 17 May 2018 12:55:19 +0200 Subject: [PATCH 19/21] Update variables expressions statement specs --- spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb index 1ceb373e19c..11e73294f18 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb @@ -100,7 +100,7 @@ describe Gitlab::Ci::Pipeline::Expression::Statement do "$PRESENT_VARIABLE =~ /^var.*/" | false "$EMPTY_VARIABLE =~ /var.*/" | false "$UNDEFINED_VARIABLE =~ /var.*/" | false - "$PRESENT_VARIABLE =~ /(?i)VAR.*/" | true + "$PRESENT_VARIABLE =~ /VAR.*/i" | true end with_them do From af9b0bfbae84a402e5c706ac29772b0d70dfa156 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 18 May 2018 10:14:10 +0200 Subject: [PATCH 20/21] Simplify untrusted regexp factory method --- lib/gitlab/untrusted_regexp.rb | 15 ++++++--------- .../pipeline/expression/lexeme/pattern_spec.rb | 4 ++-- spec/lib/gitlab/untrusted_regexp_spec.rb | 16 ++++++---------- .../malicious_regexp_shared_examples.rb | 2 ++ 4 files changed, 16 insertions(+), 21 deletions(-) diff --git a/lib/gitlab/untrusted_regexp.rb b/lib/gitlab/untrusted_regexp.rb index 70d1a7c6535..dc2d91dfa23 100644 --- a/lib/gitlab/untrusted_regexp.rb +++ b/lib/gitlab/untrusted_regexp.rb @@ -55,7 +55,7 @@ module Gitlab end def self.valid?(pattern) - self.fabricate(pattern) + !!self.fabricate(pattern) rescue RegexpError false end @@ -63,16 +63,13 @@ module Gitlab def self.fabricate(pattern) matches = pattern.match(%r{^/(?.+)/(?[ismU]*)$}) - if matches - expression = matches[:regexp] - flags = matches[:flags] + raise RegexpError, 'Invalid regular expression!' if matches.nil? - expression.prepend("(?#{flags})") if flags.present? + expression = matches[:regexp] + flags = matches[:flags] + expression.prepend("(?#{flags})") if flags.present? - self.new(expression, multiline: false) - else - self.new(pattern, multiline: false) - end + self.new(expression, multiline: false) end private diff --git a/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb index c63c38b1dbc..3ebc2e94727 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb @@ -79,7 +79,7 @@ describe Gitlab::Ci::Pipeline::Expression::Lexeme::Pattern do describe '#evaluate' do it 'returns a regular expression' do - regexp = described_class.new('abc') + regexp = described_class.new('/abc/') expect(regexp.evaluate).to eq Gitlab::UntrustedRegexp.new('abc') end @@ -87,7 +87,7 @@ describe Gitlab::Ci::Pipeline::Expression::Lexeme::Pattern do it 'raises error if evaluated regexp is not valid' do allow(Gitlab::UntrustedRegexp).to receive(:valid?).and_return(true) - regexp = described_class.new('invalid ( .*') + regexp = described_class.new('/invalid ( .*/') expect { regexp.evaluate } .to raise_error(Gitlab::Ci::Pipeline::Expression::RuntimeError) diff --git a/spec/lib/gitlab/untrusted_regexp_spec.rb b/spec/lib/gitlab/untrusted_regexp_spec.rb index 4bca320ac2c..0a6ac0aa294 100644 --- a/spec/lib/gitlab/untrusted_regexp_spec.rb +++ b/spec/lib/gitlab/untrusted_regexp_spec.rb @@ -4,9 +4,13 @@ require 'support/shared_examples/malicious_regexp_shared_examples' describe Gitlab::UntrustedRegexp do describe '.valid?' do it 'returns true if regexp is valid' do + expect(described_class.valid?('/some ( thing/')) + .to be false end it 'returns true if regexp is invalid' do + expect(described_class.valid?('/some .* thing/')) + .to be true end end @@ -32,17 +36,9 @@ describe Gitlab::UntrustedRegexp do end end - context 'when regexp is not plain pattern' do - it 'fabricates regexp without flags' do - regexp = described_class.fabricate('something') - - expect(regexp).to eq described_class.new('something') - end - end - - context 'when regexp is invalid' do + context 'when regexp is a raw pattern' do it 'raises an error' do - expect { described_class.fabricate('/some ( thing/') } + expect { described_class.fabricate('some .* thing') } .to raise_error(RegexpError) end end diff --git a/spec/support/shared_examples/malicious_regexp_shared_examples.rb b/spec/support/shared_examples/malicious_regexp_shared_examples.rb index ac5d22298bb..65026f1d7c0 100644 --- a/spec/support/shared_examples/malicious_regexp_shared_examples.rb +++ b/spec/support/shared_examples/malicious_regexp_shared_examples.rb @@ -1,3 +1,5 @@ +require 'timeout' + shared_examples 'malicious regexp' do let(:malicious_text) { 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!' } let(:malicious_regexp) { '(?i)^(([a-z])+.)+[A-Z]([a-z])+$' } From afa245142117a7e90ff6046133a2402fb8c09cb1 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 18 May 2018 10:14:19 +0200 Subject: [PATCH 21/21] Simplify pattern lexeme fabrication and matcher --- lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb b/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb index 70a221010f3..9b239c29ea4 100644 --- a/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb +++ b/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb @@ -6,7 +6,7 @@ module Gitlab require_dependency 're2' class Pattern < Lexeme::Value - PATTERN = %r{^(?/.+/[ismU]*)$}.freeze + PATTERN = %r{^/.+/[ismU]*$}.freeze def initialize(regexp) @value = regexp @@ -23,7 +23,7 @@ module Gitlab end def self.build(string) - new(string.match(PATTERN)[:regexp]) + new(string) end end end