Bugfix: Fix issue with interactive unzip on Linux (#807)

* Add new powershell commands for windows unzip

* Test fails to overwrite file

* Add new windows commands for unzip

* Add Test for failing case for both pwsh and powershell

* Modify test to confirm overwrite behavior for xar

* Delete ._test.txt

* Add fallback case for older windows systems

* Remove try

* Run Tests on windows-2016

* Update tar tests to handle existing files

* Lint

* Update tool-cache.test.ts

* Update tool-cache.test.ts

* Update tool-cache.test.ts

* Update tool-cache.test.ts

* Update from PR feedback

packages/tool-cache/__tests__/tool-cache.test.ts

@@ -239,6 +239,10 @@ describe('@actions/tool-cache', function() {
         const _7zFile: string = path.join(tempDir, 'test.7z')
         await io.cp(path.join(__dirname, 'data', 'test.7z'), _7zFile)
 
+        const destDir = path.join(tempDir, 'destination')
+        await io.mkdirP(destDir)
+        fs.writeFileSync(path.join(destDir, 'file.txt'), 'overwriteMe')
+
         // extract/cache
         const extPath: string = await tc.extract7z(_7zFile)
         await tc.cacheDir(extPath, 'my-7z-contents', '1.1.0')
@@ -247,6 +251,9 @@ describe('@actions/tool-cache', function() {
         expect(fs.existsSync(toolPath)).toBeTruthy()
         expect(fs.existsSync(`${toolPath}.complete`)).toBeTruthy()
         expect(fs.existsSync(path.join(toolPath, 'file.txt'))).toBeTruthy()
+        expect(fs.readFileSync(path.join(toolPath, 'file.txt'), 'utf8')).toBe(
+          'file.txt contents'
+        )
         expect(
           fs.existsSync(path.join(toolPath, 'file-with-ç-character.txt'))
         ).toBeTruthy()
@@ -343,6 +350,22 @@ describe('@actions/tool-cache', function() {
         await io.rmRF(tempDir)
       }
     })
+    it.each(['pwsh', 'powershell'])(
+      'unzip properly fails with bad path (%s)',
+      async powershellTool => {
+        const originalPath = process.env['PATH']
+        try {
+          if (powershellTool === 'powershell' && IS_WINDOWS) {
+            //remove pwsh from PATH temporarily to test fallback case
+            process.env['PATH'] = removePWSHFromPath(process.env['PATH'])
+          }
+
+          await expect(tc.extractZip('badPath')).rejects.toThrow()
+        } finally {
+          process.env['PATH'] = originalPath
+        }
+      }
+    )
   } else if (IS_MAC) {
     it('extract .xar', async () => {
       const tempDir = path.join(tempPath, 'test-install.xar')
@@ -356,14 +379,21 @@ describe('@actions/tool-cache', function() {
         cwd: sourcePath
       })
 
+      const destDir = path.join(tempDir, 'destination')
+      await io.mkdirP(destDir)
+      fs.writeFileSync(path.join(destDir, 'file.txt'), 'overwriteMe')
+
       // extract/cache
-      const extPath: string = await tc.extractXar(targetPath, undefined, '-x')
+      const extPath: string = await tc.extractXar(targetPath, destDir, ['-x'])
       await tc.cacheDir(extPath, 'my-xar-contents', '1.1.0')
       const toolPath: string = tc.find('my-xar-contents', '1.1.0')
 
       expect(fs.existsSync(toolPath)).toBeTruthy()
       expect(fs.existsSync(`${toolPath}.complete`)).toBeTruthy()
       expect(fs.existsSync(path.join(toolPath, 'file.txt'))).toBeTruthy()
+      expect(fs.readFileSync(path.join(toolPath, 'file.txt'), 'utf8')).toBe(
+        'file.txt contents'
+      )
       expect(
         fs.existsSync(path.join(toolPath, 'file-with-ç-character.txt'))
       ).toBeTruthy()
@@ -458,14 +488,23 @@ describe('@actions/tool-cache', function() {
     const _tgzFile: string = path.join(tempDir, 'test.tar.gz')
     await io.cp(path.join(__dirname, 'data', 'test.tar.gz'), _tgzFile)
 
+    //Create file to overwrite
+    const destDir = path.join(tempDir, 'extract-dest')
+    await io.rmRF(destDir)
+    await io.mkdirP(destDir)
+    fs.writeFileSync(path.join(destDir, 'file.txt'), 'overwriteMe')
+
     // extract/cache
-    const extPath: string = await tc.extractTar(_tgzFile)
+    const extPath: string = await tc.extractTar(_tgzFile, destDir)
     await tc.cacheDir(extPath, 'my-tgz-contents', '1.1.0')
     const toolPath: string = tc.find('my-tgz-contents', '1.1.0')
 
     expect(fs.existsSync(toolPath)).toBeTruthy()
     expect(fs.existsSync(`${toolPath}.complete`)).toBeTruthy()
     expect(fs.existsSync(path.join(toolPath, 'file.txt'))).toBeTruthy()
+    expect(fs.readFileSync(path.join(toolPath, 'file.txt'), 'utf8')).toBe(
+      'file.txt contents'
+    )
     expect(
       fs.existsSync(path.join(toolPath, 'file-with-ç-character.txt'))
     ).toBeTruthy()
@@ -496,6 +535,9 @@ describe('@actions/tool-cache', function() {
     expect(fs.existsSync(toolPath)).toBeTruthy()
     expect(fs.existsSync(`${toolPath}.complete`)).toBeTruthy()
     expect(fs.existsSync(path.join(toolPath, 'file.txt'))).toBeTruthy()
+    expect(fs.readFileSync(path.join(toolPath, 'file.txt'), 'utf8')).toBe(
+      'file.txt contents'
+    )
     expect(
       fs.existsSync(path.join(toolPath, 'file-with-ç-character.txt'))
     ).toBeTruthy()
@@ -516,8 +558,14 @@ describe('@actions/tool-cache', function() {
     const _txzFile: string = path.join(tempDir, 'test.tar.xz')
     await io.cp(path.join(__dirname, 'data', 'test.tar.xz'), _txzFile)
 
+    //Create file to overwrite
+    const destDir = path.join(tempDir, 'extract-dest')
+    await io.rmRF(destDir)
+    await io.mkdirP(destDir)
+    fs.writeFileSync(path.join(destDir, 'file.txt'), 'overwriteMe')
+
     // extract/cache
-    const extPath: string = await tc.extractTar(_txzFile, undefined, 'x')
+    const extPath: string = await tc.extractTar(_txzFile, destDir, 'x')
     await tc.cacheDir(extPath, 'my-txz-contents', '1.1.0')
     const toolPath: string = tc.find('my-txz-contents', '1.1.0')
 
@@ -530,58 +578,70 @@ describe('@actions/tool-cache', function() {
     ).toBe('foo/hello: world')
   })
 
-  it('installs a zip and finds it', async () => {
+  it.each(['pwsh', 'powershell'])(
-    const tempDir = path.join(__dirname, 'test-install-zip')
+    'installs a zip and finds it (%s)',
-    try {
+    async powershellTool => {
-      await io.mkdirP(tempDir)
+      const tempDir = path.join(__dirname, 'test-install-zip')
+      const originalPath = process.env['PATH']
+      try {
+        await io.mkdirP(tempDir)
 
-      // stage the layout for a zip file:
+        // stage the layout for a zip file:
-      //   file.txt
+        //   file.txt
-      //   folder/nested-file.txt
+        //   folder/nested-file.txt
-      const stagingDir = path.join(tempDir, 'zip-staging')
+        const stagingDir = path.join(tempDir, 'zip-staging')
-      await io.mkdirP(path.join(stagingDir, 'folder'))
+        await io.mkdirP(path.join(stagingDir, 'folder'))
-      fs.writeFileSync(path.join(stagingDir, 'file.txt'), '')
+        fs.writeFileSync(path.join(stagingDir, 'file.txt'), '')
-      fs.writeFileSync(path.join(stagingDir, 'folder', 'nested-file.txt'), '')
+        fs.writeFileSync(path.join(stagingDir, 'folder', 'nested-file.txt'), '')
 
-      // create the zip
+        // create the zip
-      const zipFile = path.join(tempDir, 'test.zip')
+        const zipFile = path.join(tempDir, 'test.zip')
-      await io.rmRF(zipFile)
+        await io.rmRF(zipFile)
-      if (IS_WINDOWS) {
+        if (IS_WINDOWS) {
-        const escapedStagingPath = stagingDir.replace(/'/g, "''") // double-up single quotes
+          const escapedStagingPath = stagingDir.replace(/'/g, "''") // double-up single quotes
-        const escapedZipFile = zipFile.replace(/'/g, "''")
+          const escapedZipFile = zipFile.replace(/'/g, "''")
-        const powershellPath =
+          const powershellPath =
-          (await io.which('pwsh', false)) ||
+            (await io.which('pwsh', false)) ||
-          (await io.which('powershell', true))
+            (await io.which('powershell', true))
-        const args = [
+          const args = [
-          '-NoLogo',
+            '-NoLogo',
-          '-Sta',
+            '-Sta',
-          '-NoProfile',
+            '-NoProfile',
-          '-NonInteractive',
+            '-NonInteractive',
-          '-ExecutionPolicy',
+            '-ExecutionPolicy',
-          'Unrestricted',
+            'Unrestricted',
-          '-Command',
+            '-Command',
-          `$ErrorActionPreference = 'Stop' ; Add-Type -AssemblyName System.IO.Compression.FileSystem ; [System.IO.Compression.ZipFile]::CreateFromDirectory('${escapedStagingPath}', '${escapedZipFile}')`
+            `$ErrorActionPreference = 'Stop' ; Add-Type -AssemblyName System.IO.Compression.FileSystem ; [System.IO.Compression.ZipFile]::CreateFromDirectory('${escapedStagingPath}', '${escapedZipFile}')`
-        ]
+          ]
-        await exec.exec(`"${powershellPath}"`, args)
+          await exec.exec(`"${powershellPath}"`, args)
-      } else {
+        } else {
-        const zipPath: string = await io.which('zip', true)
+          const zipPath: string = await io.which('zip', true)
-        await exec.exec(`"${zipPath}`, [zipFile, '-r', '.'], {cwd: stagingDir})
+          await exec.exec(`"${zipPath}`, [zipFile, '-r', '.'], {
+            cwd: stagingDir
+          })
+        }
+
+        if (powershellTool === 'powershell' && IS_WINDOWS) {
+          //remove pwsh from PATH temporarily to test fallback case
+          process.env['PATH'] = removePWSHFromPath(process.env['PATH'])
+        }
+
+        const extPath: string = await tc.extractZip(zipFile)
+        await tc.cacheDir(extPath, 'foo', '1.1.0')
+        const toolPath: string = tc.find('foo', '1.1.0')
+
+        expect(fs.existsSync(toolPath)).toBeTruthy()
+        expect(fs.existsSync(`${toolPath}.complete`)).toBeTruthy()
+        expect(fs.existsSync(path.join(toolPath, 'file.txt'))).toBeTruthy()
+        expect(
+          fs.existsSync(path.join(toolPath, 'folder', 'nested-file.txt'))
+        ).toBeTruthy()
+      } finally {
+        await io.rmRF(tempDir)
+        process.env['PATH'] = originalPath
       }
-
-      const extPath: string = await tc.extractZip(zipFile)
-      await tc.cacheDir(extPath, 'foo', '1.1.0')
-      const toolPath: string = tc.find('foo', '1.1.0')
-
-      expect(fs.existsSync(toolPath)).toBeTruthy()
-      expect(fs.existsSync(`${toolPath}.complete`)).toBeTruthy()
-      expect(fs.existsSync(path.join(toolPath, 'file.txt'))).toBeTruthy()
-      expect(
-        fs.existsSync(path.join(toolPath, 'folder', 'nested-file.txt'))
-      ).toBeTruthy()
-    } finally {
-      await io.rmRF(tempDir)
     }
-  })
+  )
 
   it('installs a zip and extracts it to specified directory', async function() {
     const tempDir = path.join(__dirname, 'test-install-zip')
@@ -593,7 +653,7 @@ describe('@actions/tool-cache', function() {
       //   folder/nested-file.txt
       const stagingDir = path.join(tempDir, 'zip-staging')
       await io.mkdirP(path.join(stagingDir, 'folder'))
-      fs.writeFileSync(path.join(stagingDir, 'file.txt'), '')
+      fs.writeFileSync(path.join(stagingDir, 'file.txt'), 'originalText')
       fs.writeFileSync(path.join(stagingDir, 'folder', 'nested-file.txt'), '')
 
       // create the zip
@@ -625,12 +685,16 @@ describe('@actions/tool-cache', function() {
       await io.rmRF(destDir)
       await io.mkdirP(destDir)
       try {
+        fs.writeFileSync(path.join(destDir, 'file.txt'), 'overwriteMe')
         const extPath: string = await tc.extractZip(zipFile, destDir)
         await tc.cacheDir(extPath, 'foo', '1.1.0')
         const toolPath: string = tc.find('foo', '1.1.0')
         expect(fs.existsSync(toolPath)).toBeTruthy()
         expect(fs.existsSync(`${toolPath}.complete`)).toBeTruthy()
         expect(fs.existsSync(path.join(toolPath, 'file.txt'))).toBeTruthy()
+        expect(fs.readFileSync(path.join(toolPath, 'file.txt'), 'utf8')).toBe(
+          'originalText'
+        )
         expect(
           fs.existsSync(path.join(toolPath, 'folder', 'nested-file.txt'))
         ).toBeTruthy()
@@ -839,3 +903,12 @@ function setGlobal<T>(key: string, value: T | undefined): void {
     g[key] = value
   }
 }
+
+function removePWSHFromPath(pathEnv: string | undefined): string {
+  return (pathEnv || '')
+    .split(';')
+    .filter(segment => {
+      return !segment.startsWith(`C:\\Program Files\\PowerShell`)
+    })
+    .join(';')
+}

packages/tool-cache/src/tool-cache.ts

@@ -272,6 +272,7 @@ export async function extractTar(
   if (isGnuTar) {
     // Suppress warnings when using GNU tar to extract archives created by BSD tar
     args.push('--warning=no-unknown-keyword')
+    args.push('--overwrite')
   }
 
   args.push('-C', destArg, '-f', fileArg)
@@ -344,21 +345,55 @@ async function extractZipWin(file: string, dest: string): Promise<void> {
   // build the powershell command
   const escapedFile = file.replace(/'/g, "''").replace(/"|\n|\r/g, '') // double-up single quotes, remove double quotes and newlines
   const escapedDest = dest.replace(/'/g, "''").replace(/"|\n|\r/g, '')
-  const command = `$ErrorActionPreference = 'Stop' ; try { Add-Type -AssemblyName System.IO.Compression.FileSystem } catch { } ; [System.IO.Compression.ZipFile]::ExtractToDirectory('${escapedFile}', '${escapedDest}')`
+  const pwshPath = await io.which('pwsh', false)
 
-  // run powershell
+  //To match the file overwrite behavior on nix systems, we use the overwrite = true flag for ExtractToDirectory
-  const powershellPath = await io.which('powershell', true)
+  //and the -Force flag for Expand-Archive as a fallback
-  const args = [
+  if (pwshPath) {
-    '-NoLogo',
+    //attempt to use pwsh with ExtractToDirectory, if this fails attempt Expand-Archive
-    '-Sta',
+    const pwshCommand = [
-    '-NoProfile',
+      `$ErrorActionPreference = 'Stop' ;`,
-    '-NonInteractive',
+      `try { Add-Type -AssemblyName System.IO.Compression.ZipFile } catch { } ;`,
-    '-ExecutionPolicy',
+      `try { [System.IO.Compression.ZipFile]::ExtractToDirectory('${escapedFile}', '${escapedDest}', $true) }`,
-    'Unrestricted',
+      `catch { if (($_.Exception.GetType().FullName -eq 'System.Management.Automation.MethodException') -or ($_.Exception.GetType().FullName -eq 'System.Management.Automation.RuntimeException') ){ Expand-Archive -LiteralPath '${escapedFile}' -DestinationPath '${escapedDest}' -Force } else { throw $_ } } ;`
-    '-Command',
+    ].join(' ')
-    command
+
-  ]
+    const args = [
-  await exec(`"${powershellPath}"`, args)
+      '-NoLogo',
+      '-NoProfile',
+      '-NonInteractive',
+      '-ExecutionPolicy',
+      'Unrestricted',
+      '-Command',
+      pwshCommand
+    ]
+
+    core.debug(`Using pwsh at path: ${pwshPath}`)
+    await exec(`"${pwshPath}"`, args)
+  } else {
+    const powershellCommand = [
+      `$ErrorActionPreference = 'Stop' ;`,
+      `try { Add-Type -AssemblyName System.IO.Compression.FileSystem } catch { } ;`,
+      `if ((Get-Command -Name Expand-Archive -Module Microsoft.PowerShell.Archive -ErrorAction Ignore)) { Expand-Archive -LiteralPath '${escapedFile}' -DestinationPath '${escapedDest}' -Force }`,
+      `else {[System.IO.Compression.ZipFile]::ExtractToDirectory('${escapedFile}', '${escapedDest}', $true) }`
+    ].join(' ')
+
+    const args = [
+      '-NoLogo',
+      '-Sta',
+      '-NoProfile',
+      '-NonInteractive',
+      '-ExecutionPolicy',
+      'Unrestricted',
+      '-Command',
+      powershellCommand
+    ]
+
+    const powershellPath = await io.which('powershell', true)
+    core.debug(`Using powershell at path: ${powershellPath}`)
+
+    await exec(`"${powershellPath}"`, args)
+  }
 }
 
 async function extractZipNix(file: string, dest: string): Promise<void> {
@@ -367,6 +402,7 @@ async function extractZipNix(file: string, dest: string): Promise<void> {
   if (!core.isDebug()) {
     args.unshift('-q')
   }
+  args.unshift('-o') //overwrite with -o, otherwise a prompt is shown which freezes the run
   await exec(`"${unzipPath}"`, args, {cwd: dest})
 }