fix: safely interpolate variables inside rules regex literals

src/utils.ts

@@ -245,6 +245,39 @@ export class Utils {
             return binary;
         };
 
+        // A `$VAR` that sits *inside* a regex literal on the RHS of `=~`/`!~`
+        // must be substituted with its value, not a quoted JS string literal.
+        // The general expansion below wraps every value in quotes (needed for
+        // `==` operands), which would otherwise leak `"` characters into the
+        // regex pattern, e.g. turning
+        //   $PHP_VERSION =~ /^\$_TARGET_PHP$/   (with _TARGET_PHP=8.3)
+        // into the malformed pattern `^\"8.3"$` instead of `^8.3$`.
+        // In a regex context GitLab still expands an escaped `\$VAR`, dropping
+        // the escaping backslash. We do this as a pre-pass, before the general
+        // expansion, so that no `$VAR` remains inside literal regexes; this
+        // leaves the "RHS is a variable that holds a regex" case (e.g.
+        // `$TAG =~ $TAG_REGEX`) to the general quoted expansion as before.
+        // The substituted value is regex-escaped so it matches literally and a
+        // value containing regex metacharacters (a branch like `feat/x`, or a
+        // value carrying `/`, `|`, `(`, ...) cannot break out of the literal,
+        // close it early, or inject expression syntax into the eval.
+        const escapeRegExp = (value: string): string => value.replace(/[.*+?^${}()|[\]\\/]/g, "\\$&");
+        const regexLiteralRhs = /(?<op>=~|!~)(?<pre>\s*["']?)\/(?<pattern>(?:\\.|[^/\\])*)\//g;
+        evalStr = evalStr.replaceAll(regexLiteralRhs, (_match, op, pre, pattern) => {
+            const expandedPattern = pattern.replaceAll(
+                /(\$\$)|\\?\$\{([a-zA-Z_]\w*)}|\\?\$([a-zA-Z_]\w*)/g,
+                (_m: string, escape: string, var1: string, var2: string) => {
+                    // Leave a `$$` escape intact for the global unescape pass
+                    // (`$$` -> literal `$`). Returning a single `$` here would
+                    // re-expose the following name as a `$VAR` to that pass and
+                    // expand it, defeating the escape.
+                    if (escape !== undefined) return "$$";
+                    return escapeRegExp(envs[var1 || var2] ?? "");
+                },
+            );
+            return `${op}${pre}/${expandedPattern}/`;
+        });
+
         // Expand all variables
         evalStr = this.expandTextWith(evalStr, {
             unescape: JSON.stringify("$"),

tests/rules-regex.test.ts

@@ -138,6 +138,87 @@ describe("gitlab rules regex", () => {
         });
 });
 
+/* eslint-disable @stylistic/quotes */
+// A `$VAR` referenced *inside* a regex literal on the RHS of `=~`/`!~` must be
+// substituted with its raw value, not a quoted JS string literal.
+const variableInRegexTests: {rule: string; envs: {[key: string]: string}; jsExpression: string; evalResult: boolean}[] = [
+    {
+        // Drupal gitlab_templates: include.drupalci.main.yml
+        rule: "$CORE_PHP_MIN == $CORE_PHP_MAX && ($PHP_VERSION =~ '/^\\$_TARGET_PHP$/' || $PHP_VERSION =~ '/^\\$CORE_PHP_MAX$/')",
+        envs: {CORE_PHP_MIN: "8.3", CORE_PHP_MAX: "8.3", _TARGET_PHP: "8.3", PHP_VERSION: "8.3"},
+        jsExpression: '"8.3" == "8.3" && ("8.3".matchRE2JS(RE2JS.compile("^8\\.3$", 0)) != null || "8.3".matchRE2JS(RE2JS.compile("^8\\.3$", 0)) != null)',
+        evalResult: true,
+    },
+    {
+        // bare $VAR inside a bare regex literal
+        rule: "$CI_COMMIT_BRANCH =~ /$BRANCHNAME/",
+        envs: {CI_COMMIT_BRANCH: "master", BRANCHNAME: "master"},
+        jsExpression: '"master".matchRE2JS(RE2JS.compile("master", 0)) != null',
+        evalResult: true,
+    },
+    {
+        // RHS is a variable that *holds* a regex literal — must still work
+        rule: "$TAG =~ $TAG_REGEX",
+        envs: {TAG: "prefix/1.0.0", TAG_REGEX: "/^prefix\\/.+/"},
+        jsExpression: '"prefix/1.0.0".matchRE2JS(RE2JS.compile("^prefix\\/.+", 0)) != null',
+        evalResult: true,
+    },
+];
+/* eslint-enable @stylistic/quotes */
+
+describe("gitlab rules regex with variable interpolation", () => {
+    variableInRegexTests.forEach((t) => {
+        test(`- if: '${t.rule}'\n\t => ${t.evalResult}`, () => {
+            const evalSpy = vi.spyOn(global, "eval");
+            const res = Utils.evaluateRuleIf(t.rule, t.envs);
+            expect(res).toBe(t.evalResult);
+            expect(evalSpy).toHaveBeenCalledWith(t.jsExpression);
+        });
+    });
+});
+
+// A variable value is escaped before it is spliced into a regex literal, so a
+// value carrying regex metacharacters matches literally and cannot break out of
+// the literal, close it early, or inject expression syntax into the eval.
+describe("gitlab rules regex with variable interpolation [escaping]", () => {
+    test("a value containing '/' matches literally and does not throw", () => {
+        // Realistic: a branch name with a slash. Previously closed the regex
+        // literal early and aborted the whole rules evaluation.
+        expect(() => Utils.evaluateRuleIf("$CI =~ /^$BRANCH$/", {CI: "feat/x", BRANCH: "feat/x"})).not.toThrow();
+        expect(Utils.evaluateRuleIf("$CI =~ /^$BRANCH$/", {CI: "feat/x", BRANCH: "feat/x"})).toBe(true);
+        expect(Utils.evaluateRuleIf("$CI =~ /^$BRANCH$/", {CI: "feat/y", BRANCH: "feat/x"})).toBe(false);
+    });
+
+    test("'/' plus operators in a value cannot inject expression syntax", () => {
+        // The LHS does not contain the literal value, so the only way this is
+        // true is injection. Must be false.
+        expect(Utils.evaluateRuleIf("$X =~ /$VAL/", {X: "no-match-here", VAL: "zzz/ || true || /x"})).toBe(false);
+    });
+
+    test("regex metacharacters in a value are matched literally", () => {
+        // `.` must not act as "any character" once it comes from a value.
+        expect(Utils.evaluateRuleIf("$X =~ /^$VAL$/", {X: "8.3", VAL: "8.3"})).toBe(true);
+        expect(Utils.evaluateRuleIf("$X =~ /^$VAL$/", {X: "8x3", VAL: "8.3"})).toBe(false);
+    });
+
+    test("!~ with a value containing '/' negates without throwing", () => {
+        expect(() => Utils.evaluateRuleIf("$X !~ /^$VAL$/", {X: "feat/y", VAL: "feat/x"})).not.toThrow();
+        expect(Utils.evaluateRuleIf("$X !~ /^$VAL$/", {X: "feat/y", VAL: "feat/x"})).toBe(true);
+        expect(Utils.evaluateRuleIf("$X !~ /^$VAL$/", {X: "feat/x", VAL: "feat/x"})).toBe(false);
+    });
+
+    test("$$ escape leaves the following name unexpanded (regression check)", () => {
+        // `$$FOO` is an escaped `$`; the name `FOO` must NOT be expanded as a
+        // variable. The pre-pass leaves `$$` for the global unescape pass
+        // exactly as before this change, so `FOO` stays literal.
+        const evalSpy = vi.spyOn(global, "eval");
+        Utils.evaluateRuleIf("$X =~ /^$$FOO$/", {X: "whatever", FOO: "bar"});
+        const compiled = evalSpy.mock.calls.at(-1)?.[0] as string;
+        expect(compiled).toContain("FOO"); // name preserved literally
+        expect(compiled).not.toContain("bar"); // FOO must NOT be expanded
+    });
+});
+
 describe("gitlab rules regex [invalid]", () => {
     tests.filter(t => t.expectedErrSubStr)
         .forEach((t) => {

tests/rules.test.ts

@@ -261,12 +261,16 @@ test.concurrent("https://github.com/firecow/gitlab-ci-local/issues/350", () => {
     rulesResult = Utils.getRulesResult({argv, cwd: "", rules, variables}, gitData);
     expect(rulesResult).toEqual({when: "manual", allowFailure: false, variables: undefined});
 
+    // A $VAR referenced inside a regex literal expands to its raw value, so
+    // /$BRANCHNAME/ becomes /master/ which matches "master". (Previously the
+    // value was wrongly quoted inside the pattern, producing /"master"/ which
+    // never matched.)
     rules = [
         {if: "$CI_COMMIT_BRANCH =~ /$BRANCHNAME/", when: "manual"},
     ];
     variables = {CI_COMMIT_BRANCH: "master", BRANCHNAME: "master"};
     rulesResult = Utils.getRulesResult({argv, cwd: "", rules, variables}, gitData);
-    expect(rulesResult).toEqual({when: "never", allowFailure: false, variables: undefined});
+    expect(rulesResult).toEqual({when: "manual", allowFailure: false, variables: undefined});
 });
 
 test.concurrent("https://github.com/firecow/gitlab-ci-local/issues/300", () => {