From 36a3a19aece0434636c4cdc339b34433c2bdaac6 Mon Sep 17 00:00:00 2001
From: Furkan Ayhan <furkanayhn@gmail.com>
Date: Mon, 18 Apr 2022 12:49:55 +0300
Subject: [PATCH 1/6] Fix CI rules-if comparison with Regexp variables

When the right side of a comparison is a regexp variable, we consider
this as a string. However, we need to try converting it to a regexp
if possible.
---
 ...les_if_comparison_with_regexp_variable.yml |  8 ++
 .../ci/pipeline/expression/lexeme/matches.rb  |  7 ++
 .../pipeline/expression/lexeme/not_matches.rb |  7 ++
 .../ci/build/rules/rule/clause/if_spec.rb     | 74 +++++++++++++++++++
 spec/lib/gitlab/ci/build/rules_spec.rb        | 13 ++++
 .../expression/lexeme/matches_spec.rb         | 28 +++++++
 .../expression/lexeme/not_matches_spec.rb     | 28 +++++++
 .../ci/pipeline/expression/statement_spec.rb  | 45 +++++++++--
 8 files changed, 205 insertions(+), 5 deletions(-)
 create mode 100644 config/feature_flags/development/ci_fix_rules_if_comparison_with_regexp_variable.yml
 create mode 100644 spec/lib/gitlab/ci/build/rules/rule/clause/if_spec.rb

diff --git a/config/feature_flags/development/ci_fix_rules_if_comparison_with_regexp_variable.yml b/config/feature_flags/development/ci_fix_rules_if_comparison_with_regexp_variable.yml
new file mode 100644
index 0000000000000000..be21707d3768ce2d
--- /dev/null
+++ b/config/feature_flags/development/ci_fix_rules_if_comparison_with_regexp_variable.yml
@@ -0,0 +1,8 @@
+---
+name: ci_fix_rules_if_comparison_with_regexp_variable
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/85310
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/359740
+milestone: '15.0'
+type: development
+group: group::pipeline authoring
+default_enabled: false
diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb b/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb
index 4d65b914d8d4442d..9d0037e70426c7a4 100644
--- a/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb
+++ b/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb
@@ -11,6 +11,13 @@ class Matches < Lexeme::LogicalOperator
             def evaluate(variables = {})
               text = @left.evaluate(variables)
               regexp = @right.evaluate(variables)
+
+              if ::Feature.enabled?(:ci_fix_rules_if_comparison_with_regexp_variable, default_enabled: :yaml)
+                # All variables are evaluated as strings, even if they are regexps.
+                # So, we need to convert them to regexps.
+                regexp = Gitlab::UntrustedRegexp::RubySyntax.fabricate(regexp) || regexp
+              end
+
               return false unless regexp
 
               regexp.scan(text.to_s).present?
diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/not_matches.rb b/lib/gitlab/ci/pipeline/expression/lexeme/not_matches.rb
index 29c5aa5d753d812d..101f5c807c6be8c0 100644
--- a/lib/gitlab/ci/pipeline/expression/lexeme/not_matches.rb
+++ b/lib/gitlab/ci/pipeline/expression/lexeme/not_matches.rb
@@ -11,6 +11,13 @@ class NotMatches < Lexeme::LogicalOperator
             def evaluate(variables = {})
               text = @left.evaluate(variables)
               regexp = @right.evaluate(variables)
+
+              if ::Feature.enabled?(:ci_fix_rules_if_comparison_with_regexp_variable, default_enabled: :yaml)
+                # All variables are evaluated as strings, even if they are regexps.
+                # So, we need to convert them to regexps.
+                regexp = Gitlab::UntrustedRegexp::RubySyntax.fabricate(regexp) || regexp
+              end
+
               return true unless regexp
 
               regexp.scan(text.to_s).empty?
diff --git a/spec/lib/gitlab/ci/build/rules/rule/clause/if_spec.rb b/spec/lib/gitlab/ci/build/rules/rule/clause/if_spec.rb
new file mode 100644
index 0000000000000000..81bce98983318938
--- /dev/null
+++ b/spec/lib/gitlab/ci/build/rules/rule/clause/if_spec.rb
@@ -0,0 +1,74 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require 'support/helpers/stubbed_feature'
+require 'support/helpers/stub_feature_flags'
+
+RSpec.describe Gitlab::Ci::Build::Rules::Rule::Clause::If do
+  include StubFeatureFlags
+
+  subject(:if_clause) { described_class.new(expression) }
+
+  describe '#satisfied_by?' do
+    let(:context_class) { Gitlab::Ci::Build::Context::Base }
+    let(:rules_context) { instance_double(context_class, variables_hash: {}) }
+
+    subject(:satisfied_by?) { if_clause.satisfied_by?(nil, rules_context) }
+
+    context 'when expression is a basic string comparison' do
+      context 'when comparison is true' do
+        let(:expression) { '"value" == "value"' }
+
+        it { is_expected.to eq(true) }
+      end
+
+      context 'when comparison is false' do
+        let(:expression) { '"value" == "other"' }
+
+        it { is_expected.to eq(false) }
+      end
+    end
+
+    context 'when expression is a regexp' do
+      context 'when comparison is true' do
+        let(:expression) { '"abcde" =~ /^ab.*/' }
+
+        it { is_expected.to eq(true) }
+      end
+
+      context 'when comparison is false' do
+        let(:expression) { '"abcde" =~ /^af.*/' }
+
+        it { is_expected.to eq(false) }
+      end
+
+      context 'when both side of the expression are variables' do
+        let(:expression) { '$teststring =~ $pattern' }
+
+        context 'when comparison is true' do
+          let(:rules_context) do
+            instance_double(context_class, variables_hash: { 'teststring' => 'abcde', 'pattern' => '/^ab.*/' })
+          end
+
+          it { is_expected.to eq(true) }
+
+          context 'when the FF ci_fix_rules_if_comparison_with_regexp_variable is disabled' do
+            before do
+              stub_feature_flags(ci_fix_rules_if_comparison_with_regexp_variable: false)
+            end
+
+            it { is_expected.to eq(false) }
+          end
+        end
+
+        context 'when comparison is false' do
+          let(:rules_context) do
+            instance_double(context_class, variables_hash: { 'teststring' => 'abcde', 'pattern' => '/^af.*/' })
+          end
+
+          it { is_expected.to eq(false) }
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/ci/build/rules_spec.rb b/spec/lib/gitlab/ci/build/rules_spec.rb
index 37bfdca4d1d75b39..e82dcd0254d24e65 100644
--- a/spec/lib/gitlab/ci/build/rules_spec.rb
+++ b/spec/lib/gitlab/ci/build/rules_spec.rb
@@ -188,6 +188,19 @@
         it { is_expected.to eq(described_class::Result.new('on_success', nil, nil, { MY_VAR: 'my var' })) }
       end
     end
+
+    context 'with a regexp variable matching rule' do
+      let(:rule_list) { [{ if: '"abcde" =~ $pattern' }] }
+
+      before do
+        allow(ci_build).to receive(:scoped_variables).and_return(
+          Gitlab::Ci::Variables::Collection.new
+            .append(key: 'pattern', value: '/^ab.*/', public: true)
+        )
+      end
+
+      it { is_expected.to eq(described_class::Result.new('on_success')) }
+    end
   end
 
   describe 'Gitlab::Ci::Build::Rules::Result' do
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 0da04d8dcf7be20c..83742699d3d6858b 100644
--- a/spec/lib/gitlab/ci/pipeline/expression/lexeme/matches_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/expression/lexeme/matches_spec.rb
@@ -1,9 +1,13 @@
 # frozen_string_literal: true
 
 require 'fast_spec_helper'
+require 'support/helpers/stubbed_feature'
+require 'support/helpers/stub_feature_flags'
 require_dependency 're2'
 
 RSpec.describe Gitlab::Ci::Pipeline::Expression::Lexeme::Matches do
+  include StubFeatureFlags
+
   let(:left) { double('left') }
   let(:right) { double('right') }
 
@@ -148,5 +152,29 @@
 
       it { is_expected.to eq(false) }
     end
+
+    context 'when right value is a regexp string' do
+      let(:right_value) { '/^ab.*/' }
+
+      context 'when matching' do
+        let(:left_value) { 'abcde' }
+
+        it { is_expected.to eq(true) }
+
+        context 'when the FF ci_fix_rules_if_comparison_with_regexp_variable is disabled' do
+          before do
+            stub_feature_flags(ci_fix_rules_if_comparison_with_regexp_variable: false)
+          end
+
+          it { is_expected.to eq(false) }
+        end
+      end
+
+      context 'when not matching' do
+        let(:left_value) { 'dfg' }
+
+        it { is_expected.to eq(false) }
+      end
+    end
   end
 end
diff --git a/spec/lib/gitlab/ci/pipeline/expression/lexeme/not_matches_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/lexeme/not_matches_spec.rb
index 9bff2355d58462c7..aad33106647b6d88 100644
--- a/spec/lib/gitlab/ci/pipeline/expression/lexeme/not_matches_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/expression/lexeme/not_matches_spec.rb
@@ -1,9 +1,13 @@
 # frozen_string_literal: true
 
 require 'fast_spec_helper'
+require 'support/helpers/stubbed_feature'
+require 'support/helpers/stub_feature_flags'
 require_dependency 're2'
 
 RSpec.describe Gitlab::Ci::Pipeline::Expression::Lexeme::NotMatches do
+  include StubFeatureFlags
+
   let(:left) { double('left') }
   let(:right) { double('right') }
 
@@ -148,5 +152,29 @@
 
       it { is_expected.to eq(true) }
     end
+
+    context 'when right value is a regexp string' do
+      let(:right_value) { '/^ab.*/' }
+
+      context 'when matching' do
+        let(:left_value) { 'abcde' }
+
+        it { is_expected.to eq(false) }
+
+        context 'when the FF ci_fix_rules_if_comparison_with_regexp_variable is disabled' do
+          before do
+            stub_feature_flags(ci_fix_rules_if_comparison_with_regexp_variable: false)
+          end
+
+          it { is_expected.to eq(true) }
+        end
+      end
+
+      context 'when not matching' do
+        let(:left_value) { 'dfg' }
+
+        it { is_expected.to eq(true) }
+      end
+    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 84713e2a798ffd5a..bbd11a00149a0151 100644
--- a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb
@@ -12,7 +12,7 @@
       .to_hash
   end
 
-  subject do
+  subject(:statement) do
     described_class.new(text, variables)
   end
 
@@ -29,6 +29,8 @@
   describe '#evaluate' do
     using RSpec::Parameterized::TableSyntax
 
+    subject(:evaluate) { statement.evaluate }
+
     where(:expression, :value) do
       '$PRESENT_VARIABLE == "my variable"'                          | true
       '"my variable" == $PRESENT_VARIABLE'                          | true
@@ -125,7 +127,7 @@
       let(:text) { expression }
 
       it "evaluates to `#{params[:value].inspect}`" do
-        expect(subject.evaluate).to eq(value)
+        expect(evaluate).to eq(value)
       end
     end
   end
@@ -133,6 +135,8 @@
   describe '#truthful?' do
     using RSpec::Parameterized::TableSyntax
 
+    subject(:truthful?) { statement.truthful? }
+
     where(:expression, :value) do
       '$PRESENT_VARIABLE == "my variable"' | true
       "$PRESENT_VARIABLE == 'no match'"    | false
@@ -151,7 +155,7 @@
       let(:text) { expression }
 
       it "returns `#{params[:value].inspect}`" do
-        expect(subject.truthful?).to eq value
+        expect(truthful?).to eq value
       end
     end
 
@@ -159,10 +163,41 @@
       let(:text) { '$PRESENT_VARIABLE' }
 
       it 'returns false' do
-        allow(subject).to receive(:evaluate)
+        allow(statement).to receive(:evaluate)
           .and_raise(described_class::StatementError)
 
-        expect(subject.truthful?).to be_falsey
+        expect(truthful?).to be_falsey
+      end
+    end
+
+    context 'when variables have patterns' do
+      let(:variables) do
+        Gitlab::Ci::Variables::Collection.new
+          .append(key: 'teststring', value: 'abcde')
+          .append(key: 'pattern1', value: '/^ab.*/')
+          .append(key: 'pattern2', value: '/^at.*/')
+          .to_hash
+      end
+
+      where(:expression, :ff, :result) do
+        '$teststring =~ "abcde"'     | true  | true
+        '$teststring =~ "abcde"'     | false | true
+        '$teststring =~ $teststring' | true  | true
+        '$teststring =~ $teststring' | false | true
+        '$teststring =~ $pattern1'   | true  | true
+        '$teststring =~ $pattern1'   | false | false
+        '$teststring =~ $pattern2'   | true  | false
+        '$teststring =~ $pattern2'   | false | false
+      end
+
+      with_them do
+        let(:text) { expression }
+
+        before do
+          stub_feature_flags(ci_fix_rules_if_comparison_with_regexp_variable: ff)
+        end
+
+        it { is_expected.to eq(result) }
       end
     end
   end
-- 
GitLab


From b197cb1e90076e6ffc92b67ee3269bfc360b4a42 Mon Sep 17 00:00:00 2001
From: Furkan Ayhan <furkanayhn@gmail.com>
Date: Thu, 21 Apr 2022 15:16:15 +0300
Subject: [PATCH 2/6] Add the documentation

---
 doc/ci/yaml/index.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/doc/ci/yaml/index.md b/doc/ci/yaml/index.md
index 1806a677c2341a6f..0fe639d4d8b08f65 100644
--- a/doc/ci/yaml/index.md
+++ b/doc/ci/yaml/index.md
@@ -3090,6 +3090,8 @@ job:
 - Unlike variables in [`script`](../variables/index.md#use-cicd-variables-in-job-scripts)
   sections, variables in rules expressions are always formatted as `$VARIABLE`.
   - You can use `rules:if` with `include` to [conditionally include other configuration files](includes.md#use-rules-with-include).
+- In [GitLab 15.0 and later](https://gitlab.com/gitlab-org/gitlab/-/issues/35438),
+  variables in the right side of `=~` and `!~` expressions are evaluated as Regexp.
 
 **Related topics**:
 
-- 
GitLab


From 4be17e0cfd648feae2c307f731087614b0d568f6 Mon Sep 17 00:00:00 2001
From: Marcel Amirault <mamirault@gitlab.com>
Date: Fri, 22 Apr 2022 08:49:36 +0000
Subject: [PATCH 3/6] Apply 1 suggestion(s) to 1 file(s)

---
 doc/ci/yaml/index.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/ci/yaml/index.md b/doc/ci/yaml/index.md
index 0fe639d4d8b08f65..ac21fdb3714099f3 100644
--- a/doc/ci/yaml/index.md
+++ b/doc/ci/yaml/index.md
@@ -3091,7 +3091,7 @@ job:
   sections, variables in rules expressions are always formatted as `$VARIABLE`.
   - You can use `rules:if` with `include` to [conditionally include other configuration files](includes.md#use-rules-with-include).
 - In [GitLab 15.0 and later](https://gitlab.com/gitlab-org/gitlab/-/issues/35438),
-  variables in the right side of `=~` and `!~` expressions are evaluated as Regexp.
+  variables on the right side of `=~` and `!~` expressions are evaluated as regular expressions.
 
 **Related topics**:
 
-- 
GitLab


From 452ecb4a51215a192b20896ce694b85cb3f3e73a Mon Sep 17 00:00:00 2001
From: Furkan Ayhan <furkanayhn@gmail.com>
Date: Wed, 27 Apr 2022 11:46:05 +0300
Subject: [PATCH 4/6] Change the comments

---
 lib/gitlab/ci/pipeline/expression/lexeme/matches.rb     | 4 ++--
 lib/gitlab/ci/pipeline/expression/lexeme/not_matches.rb | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb b/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb
index 9d0037e70426c7a4..1f4e1527fb4089fd 100644
--- a/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb
+++ b/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb
@@ -13,8 +13,8 @@ def evaluate(variables = {})
               regexp = @right.evaluate(variables)
 
               if ::Feature.enabled?(:ci_fix_rules_if_comparison_with_regexp_variable, default_enabled: :yaml)
-                # All variables are evaluated as strings, even if they are regexps.
-                # So, we need to convert them to regexps.
+                # All variables are evaluated as strings, even if they are regexp strings.
+                # So, we need to convert them to regexp objects.
                 regexp = Gitlab::UntrustedRegexp::RubySyntax.fabricate(regexp) || regexp
               end
 
diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/not_matches.rb b/lib/gitlab/ci/pipeline/expression/lexeme/not_matches.rb
index 101f5c807c6be8c0..2b4c007b12880784 100644
--- a/lib/gitlab/ci/pipeline/expression/lexeme/not_matches.rb
+++ b/lib/gitlab/ci/pipeline/expression/lexeme/not_matches.rb
@@ -13,8 +13,8 @@ def evaluate(variables = {})
               regexp = @right.evaluate(variables)
 
               if ::Feature.enabled?(:ci_fix_rules_if_comparison_with_regexp_variable, default_enabled: :yaml)
-                # All variables are evaluated as strings, even if they are regexps.
-                # So, we need to convert them to regexps.
+                # All variables are evaluated as strings, even if they are regexp strings.
+                # So, we need to convert them to regexp objects.
                 regexp = Gitlab::UntrustedRegexp::RubySyntax.fabricate(regexp) || regexp
               end
 
-- 
GitLab


From 184da5698fc55b634ef1b778122e96530803fec1 Mon Sep 17 00:00:00 2001
From: Furkan Ayhan <furkanayhn@gmail.com>
Date: Fri, 29 Apr 2022 16:04:59 +0300
Subject: [PATCH 5/6] Use lexeme pattern

---
 .../ci/pipeline/expression/lexeme/matches.rb  |  6 +--
 .../pipeline/expression/lexeme/not_matches.rb |  6 +--
 .../ci/pipeline/expression/lexeme/pattern.rb  | 18 ++++++--
 .../ci/pipeline/expression/lexeme/value.rb    |  2 +
 .../expression/lexeme/pattern_spec.rb         | 46 ++++++++++++++++++-
 5 files changed, 67 insertions(+), 11 deletions(-)

diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb b/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb
index 1f4e1527fb4089fd..ada21911f2c45123 100644
--- a/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb
+++ b/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb
@@ -12,14 +12,14 @@ def evaluate(variables = {})
               text = @left.evaluate(variables)
               regexp = @right.evaluate(variables)
 
+              return false unless regexp
+
               if ::Feature.enabled?(:ci_fix_rules_if_comparison_with_regexp_variable, default_enabled: :yaml)
                 # All variables are evaluated as strings, even if they are regexp strings.
                 # So, we need to convert them to regexp objects.
-                regexp = Gitlab::UntrustedRegexp::RubySyntax.fabricate(regexp) || regexp
+                regexp = Lexeme::Pattern.build_safe(regexp)&.evaluate(variables) || regexp
               end
 
-              return false unless regexp
-
               regexp.scan(text.to_s).present?
             end
 
diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/not_matches.rb b/lib/gitlab/ci/pipeline/expression/lexeme/not_matches.rb
index 2b4c007b12880784..4eff3e59aa4897d5 100644
--- a/lib/gitlab/ci/pipeline/expression/lexeme/not_matches.rb
+++ b/lib/gitlab/ci/pipeline/expression/lexeme/not_matches.rb
@@ -12,14 +12,14 @@ def evaluate(variables = {})
               text = @left.evaluate(variables)
               regexp = @right.evaluate(variables)
 
+              return true unless regexp
+
               if ::Feature.enabled?(:ci_fix_rules_if_comparison_with_regexp_variable, default_enabled: :yaml)
                 # All variables are evaluated as strings, even if they are regexp strings.
                 # So, we need to convert them to regexp objects.
-                regexp = Gitlab::UntrustedRegexp::RubySyntax.fabricate(regexp) || regexp
+                regexp = Lexeme::Pattern.build_safe(regexp)&.evaluate(variables) || regexp
               end
 
-              return true unless regexp
-
               regexp.scan(text.to_s).empty?
             end
 
diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb b/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb
index c7106f3ec390ea18..0a6b0ad618fa07ee 100644
--- a/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb
+++ b/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb
@@ -11,10 +11,14 @@ class Pattern < Lexeme::Value
             PATTERN = %r{^\/([^\/]|\\/)+[^\\]\/[ismU]*}.freeze
 
             def initialize(regexp)
-              super(regexp.gsub(%r{\\/}, '/'))
-
-              unless Gitlab::UntrustedRegexp::RubySyntax.valid?(@value)
-                raise Lexer::SyntaxError, 'Invalid regular expression!'
+              if regexp.is_a?(Gitlab::UntrustedRegexp)
+                super("/#{regexp.source}/")
+              else
+                super(regexp.gsub(%r{\\/}, '/'))
+
+                unless Gitlab::UntrustedRegexp::RubySyntax.valid?(@value)
+                  raise Lexer::SyntaxError, 'Invalid regular expression!'
+                end
               end
             end
 
@@ -35,6 +39,12 @@ def self.pattern
             def self.build(string)
               new(string)
             end
+
+            def self.build_safe(string)
+              new(string)
+            rescue Lexer::SyntaxError
+              nil
+            end
           end
         end
       end
diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/value.rb b/lib/gitlab/ci/pipeline/expression/lexeme/value.rb
index 6d872fee39d095eb..fa82bbe3275d9f12 100644
--- a/lib/gitlab/ci/pipeline/expression/lexeme/value.rb
+++ b/lib/gitlab/ci/pipeline/expression/lexeme/value.rb
@@ -10,6 +10,8 @@ def self.type
               :value
             end
 
+            attr_reader :value
+
             def initialize(value)
               @value = value
             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 fa4f8a20984b5c3c..91d0a8b8dc06a3bd 100644
--- a/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb
@@ -1,8 +1,40 @@
 # frozen_string_literal: true
 
-require 'spec_helper'
+require 'fast_spec_helper'
 
 RSpec.describe Gitlab::Ci::Pipeline::Expression::Lexeme::Pattern do
+  describe '#initialize' do
+    context 'when the value is a valid regular expression' do
+      it 'initializes the pattern' do
+        pattern = described_class.new('/foo/')
+
+        expect(pattern.value).to eq('/foo/')
+      end
+    end
+
+    context 'when the value is a valid regular expression with escaped slashes' do
+      it 'initializes the pattern' do
+        pattern = described_class.new('/foo\\/bar/')
+
+        expect(pattern.value).to eq('/foo/bar/')
+      end
+    end
+
+    context 'when the value is a Gitlab::UntrustedRegexp' do
+      it 'initializes the pattern' do
+        pattern = described_class.new(Gitlab::UntrustedRegexp::RubySyntax.fabricate!('/foo/'))
+
+        expect(pattern.value).to eq('/foo/')
+      end
+    end
+
+    context 'when the value is not a valid regular expression' do
+      it 'raises an error' do
+        expect { described_class.new('foo') }.to raise_error(Gitlab::Ci::Pipeline::Expression::Lexer::SyntaxError)
+      end
+    end
+  end
+
   describe '.build' do
     it 'creates a new instance of the token' do
       expect(described_class.build('/.*/'))
@@ -15,6 +47,18 @@
     end
   end
 
+  describe '.build_safe' do
+    it 'creates a new instance of the token' do
+      expect(described_class.build_safe('/.*/'))
+        .to be_a(described_class)
+    end
+
+    it 'returns nil if pattern is invalid' do
+      expect(described_class.build_safe('/ some ( thin/i'))
+        .to be_nil
+    end
+  end
+
   describe '.type' do
     it 'is a value lexeme' do
       expect(described_class.type).to eq :value
-- 
GitLab


From e9b42a6e348c39ccae2267baa718a1ce2d3f54f2 Mon Sep 17 00:00:00 2001
From: Furkan Ayhan <furkanayhn@gmail.com>
Date: Fri, 13 May 2022 13:30:22 +0300
Subject: [PATCH 6/6] Move the logic to build_and_evaluate

---
 .../ci/pipeline/expression/lexeme/matches.rb  |  4 +--
 .../pipeline/expression/lexeme/not_matches.rb |  4 +--
 .../ci/pipeline/expression/lexeme/pattern.rb  | 26 ++++++++-------
 .../expression/lexeme/pattern_spec.rb         | 33 ++++++++++---------
 4 files changed, 36 insertions(+), 31 deletions(-)

diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb b/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb
index ada21911f2c45123..6efb3a4f16a18593 100644
--- a/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb
+++ b/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb
@@ -14,10 +14,10 @@ def evaluate(variables = {})
 
               return false unless regexp
 
-              if ::Feature.enabled?(:ci_fix_rules_if_comparison_with_regexp_variable, default_enabled: :yaml)
+              if ::Feature.enabled?(:ci_fix_rules_if_comparison_with_regexp_variable)
                 # All variables are evaluated as strings, even if they are regexp strings.
                 # So, we need to convert them to regexp objects.
-                regexp = Lexeme::Pattern.build_safe(regexp)&.evaluate(variables) || regexp
+                regexp = Lexeme::Pattern.build_and_evaluate(regexp, variables)
               end
 
               regexp.scan(text.to_s).present?
diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/not_matches.rb b/lib/gitlab/ci/pipeline/expression/lexeme/not_matches.rb
index 4eff3e59aa4897d5..a72e5dbc822f5659 100644
--- a/lib/gitlab/ci/pipeline/expression/lexeme/not_matches.rb
+++ b/lib/gitlab/ci/pipeline/expression/lexeme/not_matches.rb
@@ -14,10 +14,10 @@ def evaluate(variables = {})
 
               return true unless regexp
 
-              if ::Feature.enabled?(:ci_fix_rules_if_comparison_with_regexp_variable, default_enabled: :yaml)
+              if ::Feature.enabled?(:ci_fix_rules_if_comparison_with_regexp_variable)
                 # All variables are evaluated as strings, even if they are regexp strings.
                 # So, we need to convert them to regexp objects.
-                regexp = Lexeme::Pattern.build_safe(regexp)&.evaluate(variables) || regexp
+                regexp = Lexeme::Pattern.build_and_evaluate(regexp, variables)
               end
 
               regexp.scan(text.to_s).empty?
diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb b/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb
index 0a6b0ad618fa07ee..cd4106b16bb16431 100644
--- a/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb
+++ b/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb
@@ -11,14 +11,10 @@ class Pattern < Lexeme::Value
             PATTERN = %r{^\/([^\/]|\\/)+[^\\]\/[ismU]*}.freeze
 
             def initialize(regexp)
-              if regexp.is_a?(Gitlab::UntrustedRegexp)
-                super("/#{regexp.source}/")
-              else
-                super(regexp.gsub(%r{\\/}, '/'))
-
-                unless Gitlab::UntrustedRegexp::RubySyntax.valid?(@value)
-                  raise Lexer::SyntaxError, 'Invalid regular expression!'
-                end
+              super(regexp.gsub(%r{\\/}, '/'))
+
+              unless Gitlab::UntrustedRegexp::RubySyntax.valid?(@value)
+                raise Lexer::SyntaxError, 'Invalid regular expression!'
               end
             end
 
@@ -40,10 +36,16 @@ def self.build(string)
               new(string)
             end
 
-            def self.build_safe(string)
-              new(string)
-            rescue Lexer::SyntaxError
-              nil
+            def self.build_and_evaluate(data, variables = {})
+              return data if data.is_a?(Gitlab::UntrustedRegexp)
+
+              begin
+                new_pattern = build(data)
+              rescue Lexer::SyntaxError
+                return data
+              end
+
+              new_pattern.evaluate(variables)
             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
index 91d0a8b8dc06a3bd..be205395b6940088 100644
--- a/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb
@@ -20,14 +20,6 @@
       end
     end
 
-    context 'when the value is a Gitlab::UntrustedRegexp' do
-      it 'initializes the pattern' do
-        pattern = described_class.new(Gitlab::UntrustedRegexp::RubySyntax.fabricate!('/foo/'))
-
-        expect(pattern.value).to eq('/foo/')
-      end
-    end
-
     context 'when the value is not a valid regular expression' do
       it 'raises an error' do
         expect { described_class.new('foo') }.to raise_error(Gitlab::Ci::Pipeline::Expression::Lexer::SyntaxError)
@@ -47,15 +39,26 @@
     end
   end
 
-  describe '.build_safe' do
-    it 'creates a new instance of the token' do
-      expect(described_class.build_safe('/.*/'))
-        .to be_a(described_class)
+  describe '.build_and_evaluate' do
+    context 'when the value is a valid regular expression' do
+      it 'returns the value as a Gitlab::UntrustedRegexp' do
+        expect(described_class.build_and_evaluate('/foo/'))
+          .to eq(Gitlab::UntrustedRegexp.new('foo'))
+      end
+    end
+
+    context 'when the value is a Gitlab::UntrustedRegexp' do
+      it 'returns the value itself' do
+        expect(described_class.build_and_evaluate(Gitlab::UntrustedRegexp.new('foo')))
+          .to eq(Gitlab::UntrustedRegexp.new('foo'))
+      end
     end
 
-    it 'returns nil if pattern is invalid' do
-      expect(described_class.build_safe('/ some ( thin/i'))
-        .to be_nil
+    context 'when the value is not a valid regular expression' do
+      it 'returns the value itself' do
+        expect(described_class.build_and_evaluate('foo'))
+          .to eq('foo')
+      end
     end
   end
 
-- 
GitLab