Docker and K8s: Fix shell arguments when split by the runner (#115)

* Docker: Fix shell arguments when split by the runner

* Add shlex to k8s hook as well

packages/docker/package-lock.json

@@ -12,6 +12,7 @@
         "@actions/core": "^1.9.1",
         "@actions/exec": "^1.1.1",
         "hooklib": "file:../hooklib",
+        "shlex": "^2.1.2",
         "uuid": "^8.3.2"
       },
       "devDependencies": {
@@ -4629,6 +4630,11 @@
         "node": ">=8"
       }
     },
+    "node_modules/shlex": {
+      "version": "2.1.2",
+      "resolved": "https://registry.npmjs.org/shlex/-/shlex-2.1.2.tgz",
+      "integrity": "sha512-Nz6gtibMVgYeMEhUjp2KuwAgqaJA1K155dU/HuDaEJUGgnmYfVtVZah+uerVWdH8UGnyahhDCgABbYTbs254+w=="
+    },
     "node_modules/signal-exit": {
       "version": "3.0.7",
       "resolved": "https://registry.npmjs.org/signal-exit/-/signal-exit-3.0.7.tgz",
@@ -8930,6 +8936,11 @@
       "integrity": "sha512-7++dFhtcx3353uBaq8DDR4NuxBetBzC7ZQOhmTQInHEd6bSrXdiEyzCvG07Z44UYdLShWUyXt5M/yhz8ekcb1A==",
       "dev": true
     },
+    "shlex": {
+      "version": "2.1.2",
+      "resolved": "https://registry.npmjs.org/shlex/-/shlex-2.1.2.tgz",
+      "integrity": "sha512-Nz6gtibMVgYeMEhUjp2KuwAgqaJA1K155dU/HuDaEJUGgnmYfVtVZah+uerVWdH8UGnyahhDCgABbYTbs254+w=="
+    },
     "signal-exit": {
       "version": "3.0.7",
       "resolved": "https://registry.npmjs.org/signal-exit/-/signal-exit-3.0.7.tgz",

packages/docker/package.json

@@ -13,6 +13,7 @@
     "@actions/core": "^1.9.1",
     "@actions/exec": "^1.1.1",
     "hooklib": "file:../hooklib",
+    "shlex": "^2.1.2",
     "uuid": "^8.3.2"
   },
   "devDependencies": {

packages/docker/src/dockerCommands/container.ts

@@ -444,7 +444,7 @@ export async function isContainerAlpine(containerId: string): Promise<boolean> {
     containerId,
     'sh',
     '-c',
-    "[ $(cat /etc/*release* | grep -i -e '^ID=*alpine*' -c) != 0 ] || exit 1"
+    `'[ $(cat /etc/*release* | grep -i -e "^ID=*alpine*" -c) != 0 ] || exit 1'`
   ]
   try {
     await runDockerCommand(dockerArgs)

packages/docker/src/utils.ts

@@ -5,6 +5,7 @@ import * as core from '@actions/core'
 import { env } from 'process'
 // Import this way otherwise typescript has errors
 const exec = require('@actions/exec')
+const shlex = require('shlex')
 
 export interface RunDockerCommandOptions {
   workingDir?: string
@@ -17,6 +18,7 @@ export async function runDockerCommand(
   options?: RunDockerCommandOptions
 ): Promise<string> {
   options = optionsWithDockerEnvs(options)
+  args = fixArgs(args)
   const pipes = await exec.getExecOutput('docker', args, options)
   if (pipes.exitCode !== 0) {
     core.error(`Docker failed with exit code ${pipes.exitCode}`)
@@ -84,6 +86,10 @@ export function sanitize(val: string): string {
   return newNameBuilder.join('')
 }
 
+export function fixArgs(args: string[]): string[] {
+  return shlex.split(args.join(' '))
+}
+
 export function checkEnvironment(): void {
   if (!env.GITHUB_WORKSPACE) {
     throw new Error('GITHUB_WORKSPACE is not set')

packages/docker/tests/run-script-step-test.ts

@@ -40,21 +40,36 @@ describe('run script step', () => {
     definitions.runScriptStep.args.entryPoint = '/bin/bash'
     definitions.runScriptStep.args.entryPointArgs = [
       '-c',
-      `if [[ ! $(env | grep "^PATH=") = "PATH=${definitions.runScriptStep.args.prependPath}:"* ]]; then exit 1; fi`
+      `'if [[ ! $(env | grep "^PATH=") = "PATH=${definitions.runScriptStep.args.prependPath}:"* ]]; then exit 1; fi'`
     ]
     await expect(
       runScriptStep(definitions.runScriptStep.args, prepareJobResponse.state)
     ).resolves.not.toThrow()
   })
 
+  it("Should fix expansion and print correctly in container's stdout", async () => {
+    const spy = jest.spyOn(process.stdout, 'write').mockImplementation()
+
+    definitions.runScriptStep.args.entryPoint = 'echo'
+    definitions.runScriptStep.args.entryPointArgs = ['"Mona', 'the', `Octocat"`]
+    await expect(
+      runScriptStep(definitions.runScriptStep.args, prepareJobResponse.state)
+    ).resolves.not.toThrow()
+    expect(spy).toHaveBeenCalledWith(
+      expect.stringContaining('Mona the Octocat')
+    )
+
+    spy.mockRestore()
+  })
+
   it('Should have path variable changed in container with prepend path string array', async () => {
     definitions.runScriptStep.args.prependPath = ['/some/other/path']
     definitions.runScriptStep.args.entryPoint = '/bin/bash'
     definitions.runScriptStep.args.entryPointArgs = [
       '-c',
-      `if [[ ! $(env | grep "^PATH=") = "PATH=${definitions.runScriptStep.args.prependPath.join(
+      `'if [[ ! $(env | grep "^PATH=") = "PATH=${definitions.runScriptStep.args.prependPath.join(
         ':'
-      )}:"* ]]; then exit 1; fi`
+      )}:"* ]]; then exit 1; fi'`
     ]
     await expect(
       runScriptStep(definitions.runScriptStep.args, prepareJobResponse.state)

packages/docker/tests/utils-test.ts

@@ -1,4 +1,4 @@
-import { optionsWithDockerEnvs, sanitize } from '../src/utils'
+import { optionsWithDockerEnvs, sanitize, fixArgs } from '../src/utils'
 
 describe('Utilities', () => {
   it('should return sanitized image name', () => {
@@ -10,6 +10,37 @@ describe('Utilities', () => {
     expect(sanitize(validStr)).toBe(validStr)
   })
 
+  test.each([
+    [['"Hello', 'World"'], ['Hello World']],
+    [
+      [
+        'sh',
+        '-c',
+        `'[ $(cat /etc/*release* | grep -i -e "^ID=*alpine*" -c) != 0 ] || exit 1'`
+      ],
+      [
+        'sh',
+        '-c',
+        `[ $(cat /etc/*release* | grep -i -e "^ID=*alpine*" -c) != 0 ] || exit 1`
+      ]
+    ],
+    [
+      [
+        'sh',
+        '-c',
+        `'[ $(cat /etc/*release* | grep -i -e '\\''^ID=*alpine*'\\'' -c) != 0 ] || exit 1'`
+      ],
+      [
+        'sh',
+        '-c',
+        `[ $(cat /etc/*release* | grep -i -e '^ID=*alpine*' -c) != 0 ] || exit 1`
+      ]
+    ]
+  ])('should fix split arguments(%p, %p)', (args, expected) => {
+    const got = fixArgs(args)
+    expect(got).toStrictEqual(expected)
+  })
+
   describe('with docker options', () => {
     it('should augment options with docker environment variables', () => {
       process.env.DOCKER_HOST = 'unix:///run/user/1001/docker.sock'

packages/k8s/package-lock.json

@@ -14,7 +14,8 @@
         "@actions/io": "^1.1.2",
         "@kubernetes/client-node": "^0.18.1",
         "hooklib": "file:../hooklib",
-        "js-yaml": "^4.1.0"
+        "js-yaml": "^4.1.0",
+        "shlex": "^2.1.2"
       },
       "devDependencies": {
         "@types/jest": "^27.4.1",
@@ -4163,6 +4164,11 @@
         "node": ">=8"
       }
     },
+    "node_modules/shlex": {
+      "version": "2.1.2",
+      "resolved": "https://registry.npmjs.org/shlex/-/shlex-2.1.2.tgz",
+      "integrity": "sha512-Nz6gtibMVgYeMEhUjp2KuwAgqaJA1K155dU/HuDaEJUGgnmYfVtVZah+uerVWdH8UGnyahhDCgABbYTbs254+w=="
+    },
     "node_modules/signal-exit": {
       "version": "3.0.7",
       "resolved": "https://registry.npmjs.org/signal-exit/-/signal-exit-3.0.7.tgz",
@@ -8117,6 +8123,11 @@
       "integrity": "sha512-7++dFhtcx3353uBaq8DDR4NuxBetBzC7ZQOhmTQInHEd6bSrXdiEyzCvG07Z44UYdLShWUyXt5M/yhz8ekcb1A==",
       "dev": true
     },
+    "shlex": {
+      "version": "2.1.2",
+      "resolved": "https://registry.npmjs.org/shlex/-/shlex-2.1.2.tgz",
+      "integrity": "sha512-Nz6gtibMVgYeMEhUjp2KuwAgqaJA1K155dU/HuDaEJUGgnmYfVtVZah+uerVWdH8UGnyahhDCgABbYTbs254+w=="
+    },
     "signal-exit": {
       "version": "3.0.7",
       "resolved": "https://registry.npmjs.org/signal-exit/-/signal-exit-3.0.7.tgz",

packages/k8s/package.json

@@ -18,7 +18,8 @@
     "@actions/io": "^1.1.2",
     "@kubernetes/client-node": "^0.18.1",
     "hooklib": "file:../hooklib",
-    "js-yaml": "^4.1.0"
+    "js-yaml": "^4.1.0",
+    "shlex": "^2.1.2"
   },
   "devDependencies": {
     "@types/jest": "^27.4.1",

packages/k8s/src/hooks/prepare-job.ts

@@ -23,7 +23,8 @@ import {
   generateContainerName,
   mergeContainerWithOptions,
   readExtensionFromFile,
-  PodPhase
+  PodPhase,
+  fixArgs
 } from '../k8s/utils'
 import { JOB_CONTAINER_EXTENSION_NAME, JOB_CONTAINER_NAME } from './constants'
 
@@ -206,7 +207,7 @@ export function createContainerSpec(
   }
 
   if (container.entryPointArgs?.length > 0) {
-    podContainer.args = container.entryPointArgs
+    podContainer.args = fixArgs(container.entryPointArgs)
   }
 
   podContainer.env = []

packages/k8s/src/hooks/run-container-step.ts

@@ -14,7 +14,8 @@ import {
   containerVolumes,
   PodPhase,
   mergeContainerWithOptions,
-  readExtensionFromFile
+  readExtensionFromFile,
+  fixArgs
 } from '../k8s/utils'
 import { JOB_CONTAINER_EXTENSION_NAME, JOB_CONTAINER_NAME } from './constants'
 
@@ -89,7 +90,7 @@ function createContainerSpec(
     ? [container.entryPoint]
     : undefined
   podContainer.args = container.entryPointArgs?.length
-    ? container.entryPointArgs
+    ? fixArgs(container.entryPointArgs)
     : undefined
 
   if (secretName) {

packages/k8s/src/k8s/index.ts

@@ -511,7 +511,7 @@ export async function isPodContainerAlpine(
       [
         'sh',
         '-c',
-        "[ $(cat /etc/*release* | grep -i -e '^ID=*alpine*' -c) != 0 ] || exit 1"
+        `'[ $(cat /etc/*release* | grep -i -e "^ID=*alpine*" -c) != 0 ] || exit 1'`
       ],
       podName,
       containerName

packages/k8s/src/k8s/utils.ts

@@ -7,6 +7,7 @@ import * as path from 'path'
 import { v1 as uuidv4 } from 'uuid'
 import { POD_VOLUME_NAME } from './index'
 import { JOB_CONTAINER_EXTENSION_NAME } from '../hooks/constants'
+import * as shlex from 'shlex'
 
 export const DEFAULT_CONTAINER_ENTRY_POINT_ARGS = [`-f`, `/dev/null`]
 export const DEFAULT_CONTAINER_ENTRY_POINT = 'tail'
@@ -282,3 +283,7 @@ function mergeLists<T>(base?: T[], from?: T[]): T[] {
   b.push(...from)
   return b
 }
+
+export function fixArgs(args: string[]): string[] {
+  return shlex.split(args.join(' '))
+}

packages/k8s/tests/run-container-step-test.ts

@@ -72,7 +72,7 @@ describe('Run container step', () => {
     runContainerStepData.args.entryPoint = 'bash'
     runContainerStepData.args.entryPointArgs = [
       '-c',
-      'if [[ -z $NODE_ENV ]]; then exit 1; fi'
+      "'if [[ -z $NODE_ENV ]]; then exit 1; fi'"
     ]
     await expect(
       runContainerStep(runContainerStepData.args)