Download artifact: don't extract the downloaded file if the content-type isn't a zip

.vscode/launch.json

@@ -0,0 +1,36 @@
+{
+    "version": "0.2.0",
+    "configurations": [
+      {
+        "type": "node",
+        "request": "launch",
+        "name": "Debug Jest Tests",
+        "program": "${workspaceFolder}/node_modules/jest/bin/jest.js",
+        "args": [
+          "--runInBand",
+          "--testTimeout",
+          "10000"
+        ],
+        "cwd": "${workspaceFolder}",
+        "console": "integratedTerminal",
+        "internalConsoleOptions": "neverOpen",
+        "disableOptimisticBPs": true
+      },
+      {
+        "type": "node",
+        "request": "launch",
+        "name": "Debug Current Test File",
+        "program": "${workspaceFolder}/node_modules/jest/bin/jest.js",
+        "args": [
+          "--runInBand",
+          "--testTimeout",
+          "10000",
+          "${relativeFile}"
+        ],
+        "cwd": "${workspaceFolder}",
+        "console": "integratedTerminal",
+        "internalConsoleOptions": "neverOpen",
+        "disableOptimisticBPs": true
+      }
+    ]
+}

packages/artifact/__tests__/download-artifact.test.ts

@@ -104,6 +104,7 @@ const cleanup = async (): Promise<void> => {
 const mockGetArtifactSuccess = jest.fn(() => {
   const message = new http.IncomingMessage(new net.Socket())
   message.statusCode = 200
+  message.headers['content-type'] = 'application/zip'
   message.push(fs.readFileSync(fixtures.exampleArtifact.path))
   message.push(null)
   return {
@@ -114,6 +115,7 @@ const mockGetArtifactSuccess = jest.fn(() => {
 const mockGetArtifactHung = jest.fn(() => {
   const message = new http.IncomingMessage(new net.Socket())
   message.statusCode = 200
+  message.headers['content-type'] = 'application/zip'
   // Don't push any data or call push(null) to end the stream
   // This creates a stream that hangs and never completes
   return {
@@ -134,6 +136,7 @@ const mockGetArtifactFailure = jest.fn(() => {
 const mockGetArtifactMalicious = jest.fn(() => {
   const message = new http.IncomingMessage(new net.Socket())
   message.statusCode = 200
+  message.headers['content-type'] = 'application/zip'
   message.push(fs.readFileSync(path.join(__dirname, 'fixtures', 'evil.zip'))) // evil.zip contains files that are formatted x/../../etc/hosts
   message.push(null)
   return {
@@ -623,6 +626,13 @@ describe('download-artifact', () => {
   })
 
   describe('streamExtractExternal', () => {
+    beforeEach(async () => {
+      await setup()
+      // Create workspace directory for streamExtractExternal tests
+      await fs.promises.mkdir(fixtures.workspaceDir, {recursive: true})
+    })
+    afterEach(cleanup)
+
     it('should fail if the timeout is exceeded', async () => {
       const mockSlowGetArtifact = jest.fn(mockGetArtifactHung)
 
@@ -641,12 +651,169 @@ describe('download-artifact', () => {
           {timeout: 2}
         )
         expect(true).toBe(false) // should not be called
-      } catch (e) {
+      } catch (e : any) {
         expect(e).toBeInstanceOf(Error)
         expect(e.message).toContain('did not respond in 2ms')
         expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString())
         expect(mockSlowGetArtifact).toHaveBeenCalledTimes(1)
       }
     })
+
+    it('should extract zip file when content-type is application/zip', async () => {
+      const mockHttpClient = (HttpClient as jest.Mock).mockImplementation(
+        () => {
+          return {
+            get: mockGetArtifactSuccess
+          }
+        }
+      )
+
+      await streamExtractExternal(
+        fixtures.blobStorageUrl,
+        fixtures.workspaceDir
+      )
+
+      expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString())
+      // Verify files were extracted (not saved as a single file)
+      await expectExtractedArchive(fixtures.workspaceDir)
+    })
+
+    it('should save raw file without extracting when content-type is not a zip', async () => {
+      const rawFileContent = 'This is a raw text file, not a zip'
+      const rawFileName = 'my-artifact.txt'
+
+      const mockGetRawFile = jest.fn(() => {
+        const message = new http.IncomingMessage(new net.Socket())
+        message.statusCode = 200
+        message.headers['content-type'] = 'text/plain'
+        message.headers['content-disposition'] = `attachment; filename="${rawFileName}"`
+        message.push(Buffer.from(rawFileContent))
+        message.push(null)
+        return {
+          message
+        }
+      })
+
+      const mockHttpClient = (HttpClient as jest.Mock).mockImplementation(
+        () => {
+          return {
+            get: mockGetRawFile
+          }
+        }
+      )
+
+      await streamExtractExternal(
+        fixtures.blobStorageUrl,
+        fixtures.workspaceDir
+      )
+
+      expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString())
+      // Verify file was saved as-is, not extracted
+      const savedFilePath = path.join(fixtures.workspaceDir, rawFileName)
+      expect(fs.existsSync(savedFilePath)).toBe(true)
+      expect(fs.readFileSync(savedFilePath, 'utf8')).toBe(rawFileContent)
+    })
+
+    it('should save raw file with default name when content-disposition is missing', async () => {
+      const rawFileContent = 'Binary content here'
+
+      const mockGetRawFileNoDisposition = jest.fn(() => {
+        const message = new http.IncomingMessage(new net.Socket())
+        message.statusCode = 200
+        message.headers['content-type'] = 'application/octet-stream'
+        // No content-disposition header
+        message.push(Buffer.from(rawFileContent))
+        message.push(null)
+        return {
+          message
+        }
+      })
+
+      const mockHttpClient = (HttpClient as jest.Mock).mockImplementation(
+        () => {
+          return {
+            get: mockGetRawFileNoDisposition
+          }
+        }
+      )
+
+      await streamExtractExternal(
+        fixtures.blobStorageUrl,
+        fixtures.workspaceDir
+      )
+
+      expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString())
+      // Verify file was saved with default name 'artifact'
+      const savedFilePath = path.join(fixtures.workspaceDir, 'artifact')
+      expect(fs.existsSync(savedFilePath)).toBe(true)
+      expect(fs.readFileSync(savedFilePath, 'utf8')).toBe(rawFileContent)
+    })
+
+    it('should not attempt to unzip when content-type is image/png', async () => {
+      const pngFileName = 'screenshot.png'
+      // Simple PNG header bytes for testing
+      const pngContent = Buffer.from([0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A])
+
+      const mockGetPngFile = jest.fn(() => {
+        const message = new http.IncomingMessage(new net.Socket())
+        message.statusCode = 200
+        message.headers['content-type'] = 'image/png'
+        message.headers['content-disposition'] = `attachment; filename="${pngFileName}"`
+        message.push(pngContent)
+        message.push(null)
+        return {
+          message
+        }
+      })
+
+      const mockHttpClient = (HttpClient as jest.Mock).mockImplementation(
+        () => {
+          return {
+            get: mockGetPngFile
+          }
+        }
+      )
+
+      await streamExtractExternal(
+        fixtures.blobStorageUrl,
+        fixtures.workspaceDir
+      )
+
+      expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString())
+      // Verify PNG was saved as-is
+      const savedFilePath = path.join(fixtures.workspaceDir, pngFileName)
+      expect(fs.existsSync(savedFilePath)).toBe(true)
+      expect(fs.readFileSync(savedFilePath)).toEqual(pngContent)
+    })
+
+    it('should extract when content-type is application/x-zip-compressed', async () => {
+      const mockGetZipCompressed = jest.fn(() => {
+        const message = new http.IncomingMessage(new net.Socket())
+        message.statusCode = 200
+        message.headers['content-type'] = 'application/x-zip-compressed'
+        message.push(fs.readFileSync(fixtures.exampleArtifact.path))
+        message.push(null)
+        return {
+          message
+        }
+      })
+
+      const mockHttpClient = (HttpClient as jest.Mock).mockImplementation(
+        () => {
+          return {
+            get: mockGetZipCompressed
+          }
+        }
+      )
+
+      await streamExtractExternal(
+        fixtures.blobStorageUrl,
+        fixtures.workspaceDir
+      )
+
+      expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString())
+      // Verify files were extracted
+      await expectExtractedArchive(fixtures.workspaceDir)
+    })
   })
 })

packages/artifact/src/internal/download/download-artifact.ts

@@ -1,6 +1,8 @@
 import fs from 'fs/promises'
+import * as fsSync from 'fs'
 import * as crypto from 'crypto'
 import * as stream from 'stream'
+import * as path from 'path'
 
 import * as github from '@actions/github'
 import * as core from '@actions/core'
@@ -21,6 +23,7 @@ import {
 } from '../../generated/index.js'
 import {getBackendIdsFromToken} from '../shared/util.js'
 import {ArtifactNotFoundError} from '../shared/errors.js'
+import { on } from 'events'
 
 const scrubQueryParameters = (url: string): string => {
   const parsed = new URL(url)
@@ -75,6 +78,26 @@ export async function streamExtractExternal(
     )
   }
 
+  const contentType = response.message.headers['content-type'] || ''
+  const isZip =
+    contentType === 'application/zip' ||
+    contentType === 'application/x-zip-compressed' ||
+    contentType === 'zip'
+
+  // Extract filename from Content-Disposition header
+  const contentDisposition =
+    response.message.headers['content-disposition'] || ''
+  let fileName = 'artifact'
+  const filenameMatch = contentDisposition.match(
+    /filename\*?=['"]?(?:UTF-\d['"]*)?([^;\r\n"']*)['"]?/i
+  )
+  if (filenameMatch && filenameMatch[1]) {
+    fileName = decodeURIComponent(filenameMatch[1].trim())
+  }
+
+  core.debug(`Content-Type: ${contentType}, isZip: ${isZip}`)
+  core.debug(`Content-Disposition: ${contentDisposition}, fileName: ${fileName}`)
+
   let sha256Digest: string | undefined = undefined
 
   return new Promise((resolve, reject) => {
@@ -87,37 +110,45 @@ export async function streamExtractExternal(
     }
     const timer = setTimeout(timerFn, opts.timeout)
 
+    const onError = (error: Error): void => {
+      core.debug(
+        `response.message: Artifact download failed: ${error.message}`
+      )
+      clearTimeout(timer)
+      reject(error)
+    }
+
     const hashStream = crypto.createHash('sha256').setEncoding('hex')
     const passThrough = new stream.PassThrough() 
+    .on('data', () => {
+        timer.refresh()
+      })
+    .on('error', onError)
 
     response.message.pipe(passThrough)
     passThrough.pipe(hashStream)
-    const extractStream = passThrough
 
-    extractStream
+    const onClose = (): void => {
-      .on('data', () => {
+      clearTimeout(timer)
-        timer.refresh()
+      if (hashStream) {
-      })
+        hashStream.end()
-      .on('error', (error: Error) => {
+        sha256Digest = hashStream.read() as string
-        core.debug(
+        core.info(`SHA256 digest of downloaded artifact is ${sha256Digest}`)
-          `response.message: Artifact download failed: ${error.message}`
+      }
-        )
+      resolve({sha256Digest: `sha256:${sha256Digest}`})
-        clearTimeout(timer)
+    }
-        reject(error)
+
-      })
+    if (isZip) {
-      .pipe(unzip.Extract({path: directory}))
+      // Extract zip file
-      .on('close', () => {
+      passThrough.pipe(unzip.Extract({path: directory})).on('close', onClose).on('error', onError)
-        clearTimeout(timer)
+    } else {
-        if (hashStream) {
+      // Save raw file without extracting
-          hashStream.end()
+      const filePath = path.join(directory, fileName)
-          sha256Digest = hashStream.read() as string
+      const writeStream = fsSync.createWriteStream(filePath)
-          core.info(`SHA256 digest of downloaded artifact is ${sha256Digest}`)
+
-        }
+      core.info(`Downloading raw file (non-zip) to: ${filePath}`)
-        resolve({sha256Digest: `sha256:${sha256Digest}`})
+      passThrough.pipe(writeStream).on('close', onClose).on('error', onError)
-      })
+    }
-      .on('error', (error: Error) => {
-        reject(error)
-      })
   })
 }