diff --git a/packages/artifact/__tests__/download-artifact.test.ts b/packages/artifact/__tests__/download-artifact.test.ts index e961532a..d886a55c 100644 --- a/packages/artifact/__tests__/download-artifact.test.ts +++ b/packages/artifact/__tests__/download-artifact.test.ts @@ -200,14 +200,12 @@ describe('download-artifact', () => { } ) - await expect( - downloadArtifactPublic( - fixtures.artifactID, - fixtures.repositoryOwner, - fixtures.repositoryName, - fixtures.token - ) - ).rejects.toBeInstanceOf(Error) + const response = await downloadArtifactPublic( + fixtures.artifactID, + fixtures.repositoryOwner, + fixtures.repositoryName, + fixtures.token + ) expect(downloadArtifactMock).toHaveBeenCalledWith({ owner: fixtures.repositoryOwner, @@ -223,6 +221,12 @@ describe('download-artifact', () => { expect(mockGetArtifactMalicious).toHaveBeenCalledWith( fixtures.blobStorageUrl ) + + // ensure path traversal was not possible + expect(fs.existsSync(path.join(fixtures.workspaceDir, 'x/etc/hosts'))).toBe(true); + expect(fs.existsSync(path.join(fixtures.workspaceDir, 'y/etc/hosts'))).toBe(true); + + expect(response.downloadPath).toBe(fixtures.workspaceDir) }) it('should successfully download an artifact to user defined path', async () => { diff --git a/packages/artifact/package-lock.json b/packages/artifact/package-lock.json index 643dcb86..878360b6 100644 --- a/packages/artifact/package-lock.json +++ b/packages/artifact/package-lock.json @@ -1,12 +1,12 @@ { "name": "@actions/artifact", - "version": "2.1.5", + "version": "2.1.6", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@actions/artifact", - "version": "2.1.5", + "version": "2.1.6", "license": "MIT", "dependencies": { "@actions/core": "^1.10.0", @@ -1738,9 +1738,9 @@ "integrity": "sha512-isyNax3wXoKaulPDZWHQqbmIx1k2tb9fb3GGDBRxCscfYV2Ch7WxPArBsFEG8s/safwXTT7H4QGhaIkTp9447w==" }, "node_modules/unzip-stream": { - "version": "0.3.1", - "resolved": "https://registry.npmjs.org/unzip-stream/-/unzip-stream-0.3.1.tgz", - "integrity": "sha512-RzaGXLNt+CW+T41h1zl6pGz3EaeVhYlK+rdAap+7DxW5kqsqePO8kRtWPaCiVqdhZc86EctSPVYNix30YOMzmw==", + "version": "0.3.4", + "resolved": "https://registry.npmjs.org/unzip-stream/-/unzip-stream-0.3.4.tgz", + "integrity": "sha512-PyofABPVv+d7fL7GOpusx7eRT9YETY2X04PhwbSipdj6bMxVCFJrr+nm0Mxqbf9hUiTin/UsnuFWBXlDZFy0Cw==", "dependencies": { "binary": "^0.3.0", "mkdirp": "^0.5.1" diff --git a/packages/artifact/src/internal/download/download-artifact.ts b/packages/artifact/src/internal/download/download-artifact.ts index fb7fe0a3..31965c45 100644 --- a/packages/artifact/src/internal/download/download-artifact.ts +++ b/packages/artifact/src/internal/download/download-artifact.ts @@ -1,7 +1,4 @@ import fs from 'fs/promises' -import * as stream from 'stream' -import {createWriteStream} from 'fs' -import * as path from 'path' import * as github from '@actions/github' import * as core from '@actions/core' import * as httpClient from '@actions/http-client' @@ -47,11 +44,6 @@ async function streamExtract(url: string, directory: string): Promise { await streamExtractExternal(url, directory) return } catch (error) { - if (error.message.includes('Malformed extraction path')) { - throw new Error( - `Artifact download failed with unretryable error: ${error.message}` - ) - } retryCount++ core.debug( `Failed to download artifact after ${retryCount} retries due to ${error.message}. Retrying in 5 seconds...` @@ -86,8 +78,6 @@ export async function streamExtractExternal( } const timer = setTimeout(timerFn, timeout) - const createdDirectories = new Set() - createdDirectories.add(directory) response.message .on('data', () => { timer.refresh() @@ -99,46 +89,8 @@ export async function streamExtractExternal( clearTimeout(timer) reject(error) }) - .pipe(unzip.Parse()) - .pipe( - new stream.Transform({ - objectMode: true, - transform: async (entry, _, callback) => { - const fullPath = path.normalize(path.join(directory, entry.path)) - if (!directory.endsWith(path.sep)) { - directory += path.sep - } - if (!fullPath.startsWith(directory)) { - reject(new Error(`Malformed extraction path: ${fullPath}`)) - } - - if (entry.type === 'Directory') { - if (!createdDirectories.has(fullPath)) { - createdDirectories.add(fullPath) - await resolveOrCreateDirectory(fullPath).then(() => { - entry.autodrain() - callback() - }) - } else { - entry.autodrain() - callback() - } - } else { - core.info(`Extracting artifact entry: ${fullPath}`) - if (!createdDirectories.has(path.dirname(fullPath))) { - createdDirectories.add(path.dirname(fullPath)) - await resolveOrCreateDirectory(path.dirname(fullPath)) - } - - const writeStream = createWriteStream(fullPath) - writeStream.on('finish', callback) - writeStream.on('error', reject) - entry.pipe(writeStream) - } - } - }) - ) - .on('finish', async () => { + .pipe(unzip.Extract({path: directory})) + .on('close', () => { clearTimeout(timer) resolve() })