fix: Utils.expandVariables should be pure and should not modify it's input (#1488)

Author: ANGkeith

src/utils.ts

@@ -118,28 +118,29 @@ export class Utils {
     }
 
     static expandVariables (variables: {[key: string]: string}) {
+        const _variables = {...variables}; // copy by value to prevent mutating the original input
         let expandedAnyVariables, i = 0;
         do {
             assert(i < 100, "Recursive variable expansion reached 100 iterations");
             expandedAnyVariables = false;
-            for (const [k, v] of Object.entries(variables)) {
-                const envsWithoutSelf = {...variables};
+            for (const [k, v] of Object.entries(_variables)) {
+                const envsWithoutSelf = {..._variables};
                 delete envsWithoutSelf[k];
                 // If the $$'s are converted to single $'s now, then the next
-                // iteration, they might be interpreted as variables, even
+                // iteration, they might be interpreted as _variables, even
                 // though they were *explicitly* escaped. To work around this,
                 // leave the '$$'s as the same value, then only unescape them at
                 // the very end.
-                variables[k] = Utils.expandTextWith(v, {
+                _variables[k] = Utils.expandTextWith(v, {
                     unescape: "$$",
                     variable: (name) => envsWithoutSelf[name] ?? "",
                 });
-                expandedAnyVariables ||= variables[k] !== v;
+                expandedAnyVariables ||= _variables[k] !== v;
             }
             i++;
         } while (expandedAnyVariables);
 
-        return variables;
+        return _variables;
     }
 
     static unscape$$Variables (variables: {[key: string]: string}) {

tests/test-cases/variable-expansion/.gitlab-ci-3.yml

@@ -0,0 +1,16 @@
+---
+job:
+  stage: build
+  image: busybox
+  script:
+    - echo "BUILD_IMAGE_REF=latest" > build.env
+  artifacts:
+    reports: { dotenv: build.env }
+
+job2:
+  stage: deploy
+  variables:
+    REF: $BUILD_IMAGE_REF
+  image: busybox
+  script:
+    - echo $REF

tests/test-cases/variable-expansion/integration.test.ts

@@ -89,3 +89,17 @@ test("should expand rule variables in environment", async () => {
     const filteredStdout = writeStreams.stdoutLines.filter(f => f.startsWith("job environment")).join("\n");
     expect(filteredStdout).toEqual(expected);
 });
+
+test("should expand variables referencing dotenv artifact variables", async () => {
+    const writeStreams = new WriteStreamsMock();
+    await handler({
+        cwd: "tests/test-cases/variable-expansion",
+        file: ".gitlab-ci-3.yml",
+        noColor: true,
+    }, writeStreams);
+
+    const expected = "job2 > latest";
+
+    const filteredStdout = writeStreams.stdoutLines.filter(f => f.startsWith("job2 >")).join("\n");
+    expect(filteredStdout).toEqual(expected);
+});