Repository: firecow/gitlab-ci-local (ts)

Issue:
`.gitlab-ci-local-variables.yml` not being correctly parsed.
I can't seem to get `.gitlab-ci-local-variables.yml` to be read in as variables.

I'm trying to understand how to set this up, and when running on my `.gitlab-ci.yml` file, `gitlab-ci-local` fails to run because a custom variable our yaml isn't configured.

Which is fine, as I can configure it myself.

If I go to do so through the CLI using `gitlab-ci-local --list --variable CUSTOM_VAR=value` it works fine.

Same goes for setting it up using `.gitlab-ci-local-env`

However when I configure `.gitlab-ci-local-variables.yml` as described in the readme:

```yml
---
  CUSTOM_VAR: value
```

I get an error saying that my variable isn't defined.

On debugging the code, I find that the logic for the expanded variables is the same, so I believe the values set in variablesFromFiles and expanded aren't correctly parsed by the rest of the code (since variables seem like they can be read in from multiple places, not just the unified expanded).

Or, am I misunderstanding how to use this tool?

If so, could more guidance be added to the readme please?

For now, I can use `--variable` but this won't scale for a more thorough solution :/ 



Pull request:
fix: make fileVariables accessible at an earlier stage
fixes #1333 

Patch:
diff --git a/src/job.ts b/src/job.ts
index e51c14c..cf575a0 100644
--- a/src/job.ts
+++ b/src/job.ts
@@ -153,7 +153,8 @@ export class Job {
         }
 
         const matrixVariables = opt.matrixVariables ?? {};
-        this._variables = {...globalVariables, ...jobVariables, ...matrixVariables, ...predefinedVariables, ...argvVariables};
+        const fileVariables = Utils.findEnvMatchedVariables(variablesFromFiles, this.fileVariablesDir);
+        this._variables = {...globalVariables, ...jobVariables, ...matrixVariables, ...predefinedVariables, ...fileVariables, ...argvVariables};
 
         let ciProjectDir = `${cwd}`;
         if (this.jobData["image"]) {
diff --git a/src/utils.ts b/src/utils.ts
index d6145f4..eff62d7 100644
--- a/src/utils.ts
+++ b/src/utils.ts
@@ -101,11 +101,7 @@ export class Utils {
                 } else {
                     const name = var1 || var2;
                     assert(name, "unexpected unset capture group");
-                    let value = `${expandWith.variable(name)}`;
-                    if (value.startsWith("\"/") && value.endsWith("/\"")) {
-                        value = value.substring(1).slice(0, -1);
-                    }
-                    return `${value}`;
+                    return `${expandWith.variable(name)}`;
                 }
             }
         );
@@ -205,20 +201,74 @@ export class Utils {
             unescape: JSON.stringify("$"),
             variable: (name) => JSON.stringify(envs[name] ?? null).replaceAll("\\\\", "\\"),
         });
+        const expandedEvalStr = evalStr;
+
+        // Scenario when RHS is a <regex>
+        // https://regexr.com/85sjo
+        const pattern1 = /\s*(?<operator>(?:=~)|(?:!~))\s*(?<rhs>\/.*?\/)(?<flags>[igmsuy]*)(\s|$|\))/g;
+        evalStr = evalStr.replace(pattern1, (_, operator, rhs, flags, remainingTokens) => {
+            let _operator;
+            switch (operator) {
+                case "=~":
+                    _operator = "!=";
+                    break;
+                case "!~":
+                    _operator = "==";
+                    break;
+                default:
+                    throw operator;
+            }
+            return `.match(${rhs}${flags})${remainingTokens} ${_operator} null`;
+        });
 
-        // Convert =~ to match function
-        evalStr = evalStr.replace(/\s*=~\s*(\/.*?\/[igmsuy]*)(?:\s|$)/g, ".match($1) != null");
-        evalStr = evalStr.replace(/\s*=~\s(.+?)(\)*?)(?:\s|$)/g, ".match(new RegExp($1)) != null$2"); // Without forward slashes
+        // Scenario when RHS is surrounded by double-quotes
+        // https://regexr.com/85t0g
+        const pattern2 = /\s*(?<operator>(?:=~)|(?:!~))\s*"(?<rhs>[^"\\]*(?:\\.[^"\\]*)*)"/g;
+        evalStr = evalStr.replace(pattern2, (_, operator, rhs) => {
+            let _operator;
+            switch (operator) {
+                case "=~":
+                    _operator = "!=";
+                    break;
+                case "!~":
+                    _operator = "==";
+                    break;
+                default:
+                    throw operator;
+            }
 
-        // Convert !~ to match function
-        evalStr = evalStr.replace(/\s*!~\s*(\/.*?\/[igmsuy]*)(?:\s|$)/g, ".match($1) == null");
-        evalStr = evalStr.replace(/\s*!~\s(.+?)(\)*?)(?:\s|$)/g, ".match(new RegExp($1)) == null$2"); // Without forward slashes
+            if (!/\/(.*)\/([\w]*)/.test(rhs)) {
+                throw Error(`RHS (${rhs}) must be a regex pattern. Do not rely on this behavior!
+Refer to https://docs.gitlab.com/ee/ci/jobs/job_rules.html#unexpected-behavior-from-regular-expression-matching-with- for more info...`);
+            }
+            const regex = /\/(?<pattern>.*)\/(?<flags>[igmsuy]*)/;
+            const _rhs = rhs.replace(regex, (_: string, pattern: string, flags: string) => {
+                const _pattern = pattern.replace(/(?<!\\)\//g, "\\/"); // escape potentially unescaped `/` that's in the pattern
+                return `/${_pattern}/${flags}`;
+            });
+            return `.match(new RegExp(${_rhs})) ${_operator} null`;
+        });
 
         // Convert all null.match functions to false
         evalStr = evalStr.replace(/null.match\(.+?\) != null/g, "false");
         evalStr = evalStr.replace(/null.match\(.+?\) == null/g, "false");
 
-        return Boolean(eval(evalStr));
+        let res;
+        try {
+            res = eval(evalStr);
+        } catch (err) {
+            console.log(`
+Error attempting to evaluate the following rules:
+  rules:
+    - if: '${expandedEvalStr}'
+as
+\`\`\`javascript
+${evalStr}
+\`\`\`
+`);
+            throw err;
+        }
+        return Boolean(res);
     }
 
     static evaluateRuleExist (cwd: string, ruleExists: string[] | undefined): boolean {


Test patch:
diff --git a/tests/rules-regex.test.ts b/tests/rules-regex.test.ts
new file mode 100644
index 0000000..d24f29e
--- /dev/null
+++ b/tests/rules-regex.test.ts
@@ -0,0 +1,108 @@
+
+import {Utils} from "../src/utils";
+import {GitData} from "../src/git-data";
+import {WriteStreamsMock} from "../src/write-streams";
+
+let writeStreams;
+let gitData: GitData;
+//
+beforeEach(async () => {
+    writeStreams = new WriteStreamsMock();
+    gitData = await GitData.init("tests", writeStreams);
+    jest.clearAllMocks();
+});
+
+/* eslint-disable @typescript-eslint/quotes */
+const tests = [
+    {
+        rule: '"foo" !~ /foo/',
+        jsExpression: '"foo".match(/foo/) == null',
+        evalResult: false,
+    },
+    {
+        rule: '"foo" =~ /foo/',
+        jsExpression: '"foo".match(/foo/) != null',
+        evalResult: true,
+    },
+    {
+        rule: '"foo"=~ /foo/',
+        jsExpression: '"foo".match(/foo/) != null',
+        evalResult: true,
+    },
+    {
+        rule: '"foo"=~/foo/',
+        jsExpression: '"foo".match(/foo/) != null',
+        evalResult: true,
+    },
+    {
+        rule: '"foo"=~  /foo/',
+        jsExpression: '"foo".match(/foo/) != null',
+        evalResult: true,
+    },
+    {
+        rule: '"foo" =~ "/foo/"',
+        jsExpression: '"foo".match(new RegExp(/foo/)) != null',
+        evalResult: true,
+    },
+    {
+        rule: '"test/url" =~ "/test/ur/"',
+        jsExpression: '"test/url".match(new RegExp(/test\\/ur/)) != null',
+        evalResult: true,
+    },
+    {
+        rule: '"test/url" =~ "/test\\/ur/"',
+        jsExpression: '"test/url".match(new RegExp(/test\\/ur/)) != null',
+        evalResult: true,
+    },
+    {
+        rule: '"test/url" =~ /test/ur/',
+        expectErr: true,
+    },
+    {
+        rule: '"master" =~ /master$/',
+        jsExpression: '"master".match(/master$/) != null',
+        evalResult: true,
+    },
+    {
+        rule: '"23" =~ "1234"',
+        expectErr: true,
+    },
+    {
+        rule: '"23" =~ /1234/',
+        jsExpression: '"23".match(/1234/) != null',
+        evalResult: false,
+    },
+];
+/* eslint-enable @typescript-eslint/quotes */
+
+describe("gitlab rules regex", () => {
+    tests.filter(t => !t.expectErr)
+        .forEach((t) => {
+            test(`- if: '${t.rule}'\n\t => ${t.evalResult}`, async () => {
+                const rules = [ {if: t.rule} ];
+                const evalSpy = jest.spyOn(global, "eval");
+                const evaluateRuleIfSpy = jest.spyOn(Utils, "evaluateRuleIf");
+
+                Utils.getRulesResult({cwd: "", rules, variables: {}}, gitData);
+                expect(evalSpy).toHaveBeenCalledWith(t.jsExpression);
+                expect(evaluateRuleIfSpy).toHaveReturnedWith(t.evalResult);
+            });
+        });
+});
+
+describe("gitlab rules regex [invalid]", () => {
+    tests.filter(t => t.expectErr)
+        .forEach((t) => {
+            test(`- if: '${t.rule}'\n\t => error`, async () => {
+                const rules = [ {if: t.rule} ];
+
+                try {
+                    Utils.getRulesResult({cwd: "", rules, variables: {}}, gitData);
+                } catch (e) {
+                    return;
+                }
+
+                throw new Error("Error is expected but not thrown/caught");
+            });
+        });
+});
diff --git a/tests/rules.test.ts b/tests/rules.test.ts
index 1f2bb90..c1936ac 100644
--- a/tests/rules.test.ts
+++ b/tests/rules.test.ts
@@ -264,16 +264,16 @@ test("https://github.com/firecow/gitlab-ci-local/issues/350", () => {
 test("https://github.com/firecow/gitlab-ci-local/issues/300", () => {
     let rules, rulesResult, variables;
     rules = [
-        {if: "$VAR1 && (($VAR3 =~ /ci-skip-job-/ && $VAR2 =~ $VAR3) || ($VAR3 =~ /ci-skip-stage-/ && $VAR2 =~ $VAR3))", when: "manual"},
+        {if: "$VAR1 && (($VAR2 =~ /ci-skip-job-/ && $VAR2 =~ $VAR3) || ($VAR2 =~ /ci-skip-stage-/ && $VAR2 =~ $VAR3))", when: "manual"},
     ];
-    variables = {VAR1: "val", VAR2: "ci-skip-job-", VAR3: "ci-skip-job-"};
+    variables = {VAR1: "val", VAR2: "ci-skip-job-", VAR3: "/ci-skip-job-/"};
     rulesResult = Utils.getRulesResult({cwd: "", rules, variables}, gitData);
     expect(rulesResult).toEqual({when: "manual", allowFailure: false, variables: undefined});
 
     rules = [
-        {if: "$VAR1 && (($VAR3 =~ /ci-skip-job-/ && $VAR2 =~ $VAR3) || ($VAR3 =~ /ci-skip-stage-/ && $VAR2 =~ $VAR3))", when: "manual"},
+        {if: "$VAR1 && (($VAR2 =~ /ci-skip-job-/ && $VAR2 =~ $VAR3) || ($VAR2 =~ /ci-skip-stage-/ && $VAR2 =~ $VAR3))", when: "manual"},
     ];
-    variables = {VAR1: "val", VAR2: "ci-skip-stage-", VAR3: "ci-skip-stage-"};
+    variables = {VAR1: "val", VAR2: "ci-skip-stage-", VAR3: "/ci-skip-stage-/"};
     rulesResult = Utils.getRulesResult({cwd: "", rules, variables}, gitData);
     expect(rulesResult).toEqual({when: "manual", allowFailure: false, variables: undefined});
 });
diff --git a/tests/test-cases/gitlab-runner-emulation/integration.emulate-runner.test.ts b/tests/test-cases/gitlab-runner-emulation/integration.emulate-runner.test.ts
index dd11a4d..0b92cf7 100644
--- a/tests/test-cases/gitlab-runner-emulation/integration.emulate-runner.test.ts
+++ b/tests/test-cases/gitlab-runner-emulation/integration.emulate-runner.test.ts
@@ -2,12 +2,14 @@ import {WriteStreamsMock} from "../../../src/write-streams";
 import {handler} from "../../../src/handler";
 import {initBashSpy, initSpawnSpy} from "../../mocks/utils.mock";
 import {WhenStatics} from "../../mocks/when-statics";
+import {cleanupJobResources, Job} from "../../../src/job";
 import {
     GitlabRunnerCPUsPresetValue,
     GitlabRunnerMemoryPresetValue,
     GitlabRunnerPresetValues,
 } from "../../../src/gitlab-preset";
 
+let jobs: Job[] = [];
 beforeAll(() => {
     initSpawnSpy([...WhenStatics.all, {
         cmdArgs: ["docker", "cp", expect.any(String), expect.any(String)],
@@ -15,13 +17,21 @@ beforeAll(() => {
     }]);
 });
 
+beforeEach(async () => {
+    jobs = [];
+});
+
+afterEach(async () => {
+    await cleanupJobResources(jobs);
+});
+
 test("--container-emulate some_unexisting_runner", async () => {
     try {
         const writeStreams = new WriteStreamsMock();
         await handler({
             cwd: "tests/test-cases/container-executable",
             containerEmulate: "some_unexisting_runner",
-        }, writeStreams);
+        }, writeStreams, jobs);
 
         expect(true).toBe(false);
     } catch (e: unknown) {
diff --git a/tests/test-cases/network-arg/integration.network-arg.test.ts b/tests/test-cases/network-arg/integration.network-arg.test.ts
index 9a7ca87..ee63715 100644
--- a/tests/test-cases/network-arg/integration.network-arg.test.ts
+++ b/tests/test-cases/network-arg/integration.network-arg.test.ts
@@ -1,15 +1,24 @@
 import {WriteStreamsMock} from "../../../src/write-streams";
 import {handler} from "../../../src/handler";
+import {Job, cleanupJobResources} from "../../../src/job";
 import chalk from "chalk";
 import {initSpawnSpy, initBashSpy} from "../../mocks/utils.mock";
 import {WhenStatics} from "../../mocks/when-statics";
 import assert from "assert";
 import {AssertionError} from "assert";
 
+let jobs: Job[] = [];
 beforeAll(() => {
     initSpawnSpy(WhenStatics.all);
 });
 
+beforeEach(async () => {
+    jobs = [];
+});
+
+afterEach(async () => {
+    await cleanupJobResources(jobs);
+});
 
 test("network-host <test-job>", async () => {
     const bashSpy = initBashSpy([]);
@@ -36,7 +45,7 @@ test("network-host <service-job>", async () => {
             cwd: "tests/test-cases/network-arg",
             job: ["service-job"],
             network: ["host"],
-        }, writeStreams);
+        }, writeStreams, jobs);
     } catch (e) {
         assert(e instanceof AssertionError, "e is not instanceof AssertionError");
         expect(e.message).toBe(chalk`Cannot add service network alias with network mode 'host'`);
@@ -68,7 +77,7 @@ test("network-none <service-job>", async () => {
             cwd: "tests/test-cases/network-arg",
             job: ["service-job"],
             network: ["none"],
-        }, writeStreams);
+        }, writeStreams, jobs);
     } catch (e) {
         assert(e instanceof AssertionError, "e is not instanceof AssertionError");
         expect(e.message).toBe(chalk`Cannot add service network alias with network mode 'none'`);
diff --git a/tests/test-cases/project-variables-file/.gitlab-ci-issue-1333.yml b/tests/test-cases/project-variables-file/.gitlab-ci-issue-1333.yml
new file mode 100644
index 0000000..917150e
--- /dev/null
+++ b/tests/test-cases/project-variables-file/.gitlab-ci-issue-1333.yml
@@ -0,0 +1,6 @@
+---
+issue-1333:
+  script:
+    - echo "$API_KEY"
+  environment:
+    name: $CUSTOM_VAR
diff --git a/tests/test-cases/project-variables-file/.gitlab-ci-local-variables.yml b/tests/test-cases/project-variables-file/.gitlab-ci-local-variables.yml
index 9c47c8d..a0e347b 100644
--- a/tests/test-cases/project-variables-file/.gitlab-ci-local-variables.yml
+++ b/tests/test-cases/project-variables-file/.gitlab-ci-local-variables.yml
@@ -14,3 +14,9 @@ FUNNY2:
 FUNNY3:
   values:
     "*": Recursive CI/CD
+
+CUSTOM_VAR: prod
+
+API_KEY:
+  values:
+    "prod": firecow
diff --git a/tests/test-cases/project-variables-file/integration.project-variables-file.test.ts b/tests/test-cases/project-variables-file/integration.project-variables-file.test.ts
index 6f0d60e..1ee6f17 100644
--- a/tests/test-cases/project-variables-file/integration.project-variables-file.test.ts
+++ b/tests/test-cases/project-variables-file/integration.project-variables-file.test.ts
@@ -21,3 +21,16 @@ test.concurrent("project-variables-file <test-job>", async () => {
     ];
     expect(writeStreams.stdoutLines).toEqual(expect.arrayContaining(expected));
 });
+
+test.concurrent("project-variables-file <issue-1333>", async () => {
+    const writeStreams = new WriteStreamsMock();
+    await handler({
+        cwd: "tests/test-cases/project-variables-file",
+        file: ".gitlab-ci-issue-1333.yml",
+    }, writeStreams);
+
+    const expected = [
+        chalk`{blueBright issue-1333} {greenBright >} firecow`,
+    ];
+    expect(writeStreams.stdoutLines).toEqual(expect.arrayContaining(expected));
+});