Escape variables in image entrypoints (#1573)

Signed-off-by: Jonathan GAYVALLET <[email protected]>

Author: Meallia

src/job.ts

@@ -911,19 +911,15 @@ export class Job {
             }
 
             if (this.imageEntrypoint) {
-                if (this.imageEntrypoint[0] == "") {
-                    dockerCmd += "--entrypoint '' ";
-                } else {
-                    dockerCmd += `--entrypoint ${this.imageEntrypoint[0]} `;
-                }
+                dockerCmd += `--entrypoint ${Utils.safeBashString(this.imageEntrypoint[0])} `;
             }
 
             dockerCmd += `${(await this.mountCacheCmd(writeStreams, expanded)).join(" ")} `;
             dockerCmd += `${imageName} `;
 
             if (this.imageEntrypoint?.length ?? 0 > 1) {
                 this.imageEntrypoint?.slice(1).forEach((e) => {
-                    dockerCmd += `"${e}" `;
+                    dockerCmd += `${Utils.safeBashString(e)} `;
                 });
             }
 
@@ -1446,11 +1442,7 @@ export class Job {
 
         const serviceEntrypoint = service.entrypoint;
         if (serviceEntrypoint) {
-            if (serviceEntrypoint[0] == "") {
-                dockerCmd += "--entrypoint '' ";
-            } else {
-                dockerCmd += `--entrypoint ${serviceEntrypoint[0]} `;
-            }
+            dockerCmd += `--entrypoint ${Utils.safeBashString(serviceEntrypoint[0])} `;
         }
         dockerCmd += `--volume ${this.buildVolumeName}:${this.ciProjectDir} `;
         dockerCmd += `--volume ${this.tmpVolumeName}:${this.fileVariablesDir} `;
@@ -1469,9 +1461,7 @@ export class Job {
 
         if (serviceEntrypoint?.length ?? 0 > 1) {
             serviceEntrypoint?.slice(1).forEach((e) => {
-                dockerCmd += `'${e
-                    .replace(/'/g, "'\"'\"'") // replaces `'` with `'"'"'`
-                }' `;
+                dockerCmd += `${Utils.safeBashString(e)} `;
             });
         }
         (service.command ?? []).forEach((e) => dockerCmd += `"${e.replace(/\$/g, "\\$")}" `);

src/utils.ts

@@ -53,6 +53,10 @@ export class Utils {
         });
     }
 
+    static safeBashString (s: string) {
+        return `'${s.replace(/'/g, "'\"'\"'")}'`; // replaces `'` with `'"'"'`
+    }
+
     static forEachRealJob (gitlabData: any, callback: (jobName: string, jobData: any) => void) {
         for (const [jobName, jobData] of Object.entries<any>(gitlabData)) {
             if (Job.illegalJobNames.has(jobName) || jobName[0].startsWith(".")) {

tests/test-cases/image/.gitlab-ci.yml

@@ -63,3 +63,17 @@ image-user:
       user: nobody
   script:
     - id -u
+
+image-entrypoint-with-variables:
+  variables:
+    FOO: BAR
+  image:
+    name: alpine
+    entrypoint:
+      - sh
+      - -c
+      - |
+        if [ -z "$FOO" ]; then >&2 echo FOO is not defined in entrypoint; exit 1; fi
+        sh "$@"
+  script:
+    - echo "success"

tests/test-cases/image/integration.test.ts

@@ -135,3 +135,17 @@ test.concurrent("pull invalid image", async () => {
 
     await cleanupJobResources(jobs);
 });
+
+test.concurrent("no variable substitution in entrpoint", async () => {
+    const writeStreams = new WriteStreamsMock();
+
+    await handler({
+        cwd: "tests/test-cases/image",
+        job: ["image-entrypoint-with-variables"],
+    }, writeStreams);
+
+    const expected = [
+        chalk`{blueBright image-entrypoint-with-variables} {greenBright >} success`,
+    ];
+    expect(writeStreams.stdoutLines).toEqual(expect.arrayContaining(expected));
+});