Skip to content

Commit

Permalink
Merge pull request #1724 from actions/bethanyj28/update-unzip-stream
Browse files Browse the repository at this point in the history
Use latest `unzip-stream` and `unzip.Extract`
  • Loading branch information
bethanyj28 committed Apr 24, 2024
2 parents d82fd09 + 9eb3d3a commit 29885a8
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 64 deletions.
4 changes: 4 additions & 0 deletions packages/artifact/RELEASES.md
@@ -1,5 +1,9 @@
# @actions/artifact Releases

### 2.1.7

- Update unzip-stream dependency and reverted to using `unzip.Extract()`

### 2.1.6

- Will retry on invalid request responses.
Expand Down
24 changes: 16 additions & 8 deletions packages/artifact/__tests__/download-artifact.test.ts
Expand Up @@ -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,
Expand All @@ -223,6 +221,16 @@ 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 () => {
Expand Down
10 changes: 5 additions & 5 deletions packages/artifact/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/artifact/package.json
@@ -1,6 +1,6 @@
{
"name": "@actions/artifact",
"version": "2.1.6",
"version": "2.1.7",
"preview": true,
"description": "Actions artifact lib",
"keywords": [
Expand Down
52 changes: 2 additions & 50 deletions 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'
Expand Down Expand Up @@ -47,11 +44,6 @@ async function streamExtract(url: string, directory: string): Promise<void> {
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...`
Expand Down Expand Up @@ -86,8 +78,6 @@ export async function streamExtractExternal(
}
const timer = setTimeout(timerFn, timeout)

const createdDirectories = new Set<string>()
createdDirectories.add(directory)
response.message
.on('data', () => {
timer.refresh()
Expand All @@ -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()
})
Expand Down

0 comments on commit 29885a8

Please sign in to comment.