From c9af6bb1b3ae4728501b2b1ae42adcf3c3888977 Mon Sep 17 00:00:00 2001
From: Thomas Boop <52323235+thboop@users.noreply.github.com>
Date: Mon, 7 Jun 2021 14:16:16 -0400
Subject: [PATCH] Update escaping rules in io's rmRF (#828)

* Better Handling of escaping in rmrf
---
 packages/io/__tests__/io.test.ts | 39 ++++++++++++++++++++++++++++++++
 packages/io/src/io-util.ts       |  5 ++++
 packages/io/src/io.ts            | 20 +++++++++++++---
 3 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/packages/io/__tests__/io.test.ts b/packages/io/__tests__/io.test.ts
index 08bf5e677d..0f1b38429f 100644
--- a/packages/io/__tests__/io.test.ts
+++ b/packages/io/__tests__/io.test.ts
@@ -556,6 +556,45 @@ describe('rmRF', () => {
       await assertNotExists(symlinkFile)
       await assertNotExists(outerDirectory)
     })
+  } else {
+    it('correctly escapes % on windows', async () => {
+      const root: string = path.join(getTestTemp(), 'rmRF_escape_test_win')
+      const directory: string = path.join(root, '%test%')
+      await io.mkdirP(root)
+      await io.mkdirP(directory)
+      const oldEnv = process.env['test']
+      process.env['test'] = 'thisshouldnotresolve'
+
+      await io.rmRF(directory)
+      await assertNotExists(directory)
+      process.env['test'] = oldEnv
+    })
+
+    it('Should throw for invalid characters', async () => {
+      const root: string = path.join(getTestTemp(), 'rmRF_invalidChar_Windows')
+      const errorString =
+        'File path must not contain `*`, `"`, `<`, `>` or `|` on Windows'
+      await expect(io.rmRF(path.join(root, '"'))).rejects.toHaveProperty(
+        'message',
+        errorString
+      )
+      await expect(io.rmRF(path.join(root, '<'))).rejects.toHaveProperty(
+        'message',
+        errorString
+      )
+      await expect(io.rmRF(path.join(root, '>'))).rejects.toHaveProperty(
+        'message',
+        errorString
+      )
+      await expect(io.rmRF(path.join(root, '|'))).rejects.toHaveProperty(
+        'message',
+        errorString
+      )
+      await expect(io.rmRF(path.join(root, '*'))).rejects.toHaveProperty(
+        'message',
+        errorString
+      )
+    })
   }
 
   it('removes symlink folder with missing source using rmRF', async () => {
diff --git a/packages/io/src/io-util.ts b/packages/io/src/io-util.ts
index b09ead49b6..79674762e9 100644
--- a/packages/io/src/io-util.ts
+++ b/packages/io/src/io-util.ts
@@ -166,3 +166,8 @@ function isUnixExecutable(stats: fs.Stats): boolean {
     ((stats.mode & 64) > 0 && stats.uid === process.getuid())
   )
 }
+
+// Get the path of cmd.exe in windows
+export function getCmdPath(): string {
+  return process.env['COMSPEC'] ?? `cmd.exe`
+}
diff --git a/packages/io/src/io.ts b/packages/io/src/io.ts
index e75e14f775..a6c31ab6ee 100644
--- a/packages/io/src/io.ts
+++ b/packages/io/src/io.ts
@@ -5,6 +5,7 @@ import {promisify} from 'util'
 import * as ioUtil from './io-util'
 
 const exec = promisify(childProcess.exec)
+const execFile = promisify(childProcess.execFile)
 
 /**
  * Interface for cp/mv options
@@ -117,11 +118,24 @@ export async function rmRF(inputPath: string): Promise<void> {
   if (ioUtil.IS_WINDOWS) {
     // Node doesn't provide a delete operation, only an unlink function. This means that if the file is being used by another
     // program (e.g. antivirus), it won't be deleted. To address this, we shell out the work to rd/del.
+
+    // Check for invalid characters
+    // https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file
+    if (/[*"<>|]/.test(inputPath)) {
+      throw new Error(
+        'File path must not contain `*`, `"`, `<`, `>` or `|` on Windows'
+      )
+    }
     try {
+      const cmdPath = ioUtil.getCmdPath()
       if (await ioUtil.isDirectory(inputPath, true)) {
-        await exec(`rd /s /q "${inputPath}"`)
+        await exec(`${cmdPath} /s /c "rd /s /q "%inputPath%""`, {
+          env: {inputPath}
+        })
       } else {
-        await exec(`del /f /a "${inputPath}"`)
+        await exec(`${cmdPath} /s /c "del /f /a "%inputPath%""`, {
+          env: {inputPath}
+        })
       }
     } catch (err) {
       // if you try to delete a file that doesn't exist, desired result is achieved
@@ -149,7 +163,7 @@ export async function rmRF(inputPath: string): Promise<void> {
     }
 
     if (isDir) {
-      await exec(`rm -rf "${inputPath}"`)
+      await execFile(`rm`, [`-rf`, `${inputPath}`])
     } else {
       await ioUtil.unlink(inputPath)
     }