k8s: handle $ symbols in environment variable names and values (#74)

* Add test cases * Handle $ symbols in environment variable names and values
actions · Apr 18, 2023 · c37c5ca · c37c5ca
1 parent 04b58be
commit c37c5ca
Show file tree

Hide file tree

Showing 3 changed files with 108 additions and 3 deletions.
diff --git a/packages/k8s/src/k8s/utils.ts b/packages/k8s/src/k8s/utils.ts
@@ -111,13 +111,21 @@ export function writeEntryPointScript(
   if (environmentVariables && Object.entries(environmentVariables).length) {
     const envBuffer: string[] = []
     for (const [key, value] of Object.entries(environmentVariables)) {
-      if (key.includes(`=`) || key.includes(`'`) || key.includes(`"`)) {
+      if (
+        key.includes(`=`) ||
+        key.includes(`'`) ||
+        key.includes(`"`) ||
+        key.includes(`$`)
+      ) {
         throw new Error(
-          `environment key ${key} is invalid - the key must not contain =, ' or "`
+          `environment key ${key} is invalid - the key must not contain =, $, ', or "`
         )
       }
       envBuffer.push(
-        `"${key}=${value.replace(/\\/g, '\\\\').replace(/"/g, '\\"')}"`
+        `"${key}=${value
+          .replace(/\\/g, '\\\\')
+          .replace(/"/g, '\\"')
+          .replace(/\$/g, '\\$')}"`
       )
     }
     environmentPrefix = `env ${envBuffer.join(' ')} `

diff --git a/packages/k8s/tests/k8s-utils-test.ts b/packages/k8s/tests/k8s-utils-test.ts
@@ -49,6 +49,81 @@ describe('k8s utils', () => {
       ).toThrow()
     })
 
+    it('should throw if environment variable name contains double quote', () => {
+      expect(() =>
+        writeEntryPointScript(
+          '/test',
+          'sh',
+          ['-e', 'script.sh'],
+          ['/prepend/path'],
+          {
+            'SOME"_ENV': 'SOME_VALUE'
+          }
+        )
+      ).toThrow()
+    })
+
+    it('should throw if environment variable name contains =', () => {
+      expect(() =>
+        writeEntryPointScript(
+          '/test',
+          'sh',
+          ['-e', 'script.sh'],
+          ['/prepend/path'],
+          {
+            'SOME=ENV': 'SOME_VALUE'
+          }
+        )
+      ).toThrow()
+    })
+
+    it('should throw if environment variable name contains single quote', () => {
+      expect(() =>
+        writeEntryPointScript(
+          '/test',
+          'sh',
+          ['-e', 'script.sh'],
+          ['/prepend/path'],
+          {
+            "SOME'_ENV": 'SOME_VALUE'
+          }
+        )
+      ).toThrow()
+    })
+
+    it('should throw if environment variable name contains dollar', () => {
+      expect(() =>
+        writeEntryPointScript(
+          '/test',
+          'sh',
+          ['-e', 'script.sh'],
+          ['/prepend/path'],
+          {
+            SOME_$_ENV: 'SOME_VALUE'
+          }
+        )
+      ).toThrow()
+    })
+
+    it('should escape double quote, dollar and backslash in environment variable values', () => {
+      const { runnerPath } = writeEntryPointScript(
+        '/test',
+        'sh',
+        ['-e', 'script.sh'],
+        ['/prepend/path'],
+        {
+          DQUOTE: '"',
+          BACK_SLASH: '\\',
+          DOLLAR: '$'
+        }
+      )
+      expect(fs.existsSync(runnerPath)).toBe(true)
+      const script = fs.readFileSync(runnerPath, 'utf8')
+      expect(script).toContain('"DQUOTE=\\"')
+      expect(script).toContain('"BACK_SLASH=\\\\"')
+      expect(script).toContain('"DOLLAR=\\$"')
+    })
+
     it('should return object with containerPath and runnerPath', () => {
       const { containerPath, runnerPath } = writeEntryPointScript(
         '/test',

diff --git a/packages/k8s/tests/run-script-step-test.ts b/packages/k8s/tests/run-script-step-test.ts
@@ -89,6 +89,28 @@ describe('Run script step', () => {
     ).resolves.not.toThrow()
   })
 
+  it('Dollar symbols in environment variables should not be expanded', async () => {
+    runScriptStepDefinition.args.environmentVariables = {
+      VARIABLE1: '$VAR',
+      VARIABLE2: '${VAR}',
+      VARIABLE3: '$(VAR)'
+    }
+    runScriptStepDefinition.args.entryPointArgs = [
+      '-c',
+      '\'if [[ -z "$VARIABLE1" ]]; then exit 1; fi\'',
+      '\'if [[ -z "$VARIABLE2" ]]; then exit 2; fi\'',
+      '\'if [[ -z "$VARIABLE3" ]]; then exit 3; fi\''
+    ]
+
+    await expect(
+      runScriptStep(
+        runScriptStepDefinition.args,
+        prepareJobOutputData.state,
+        null
+      )
+    ).resolves.not.toThrow()
+  })
+
   it('Should have path variable changed in container with prepend path string array', async () => {
     runScriptStepDefinition.args.prependPath = ['/some/other/path']
     runScriptStepDefinition.args.entryPoint = '/bin/bash'