Skip to content

Commit

Permalink
use latest unzip-stream
Browse files Browse the repository at this point in the history
  • Loading branch information
bethanyj28 committed Apr 23, 2024
1 parent d82fd09 commit 476276b
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 63 deletions.
20 changes: 12 additions & 8 deletions packages/artifact/__tests__/download-artifact.test.ts
Original file line number Diff line number Diff line change
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,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);

Check failure on line 226 in packages/artifact/__tests__/download-artifact.test.ts

View workflow job for this annotation

GitHub Actions / Build (ubuntu-latest)

Replace `fs.existsSync(path.join(fixtures.workspaceDir,·'x/etc/hosts'))).toBe(true);` with `⏎········fs.existsSync(path.join(fixtures.workspaceDir,·'x/etc/hosts'))⏎······).toBe(true)`

Check failure on line 226 in packages/artifact/__tests__/download-artifact.test.ts

View workflow job for this annotation

GitHub Actions / Build (ubuntu-latest)

Extra semicolon

Check failure on line 226 in packages/artifact/__tests__/download-artifact.test.ts

View workflow job for this annotation

GitHub Actions / Build (macos-latest)

Replace `fs.existsSync(path.join(fixtures.workspaceDir,·'x/etc/hosts'))).toBe(true);` with `⏎········fs.existsSync(path.join(fixtures.workspaceDir,·'x/etc/hosts'))⏎······).toBe(true)`

Check failure on line 226 in packages/artifact/__tests__/download-artifact.test.ts

View workflow job for this annotation

GitHub Actions / Build (macos-latest)

Extra semicolon

Check failure on line 226 in packages/artifact/__tests__/download-artifact.test.ts

View workflow job for this annotation

GitHub Actions / Build (windows-latest)

Replace `fs.existsSync(path.join(fixtures.workspaceDir,·'x/etc/hosts'))).toBe(true);` with `␍⏎········fs.existsSync(path.join(fixtures.workspaceDir,·'x/etc/hosts'))␍⏎······).toBe(true)`

Check failure on line 226 in packages/artifact/__tests__/download-artifact.test.ts

View workflow job for this annotation

GitHub Actions / Build (windows-latest)

Extra semicolon
expect(fs.existsSync(path.join(fixtures.workspaceDir, 'y/etc/hosts'))).toBe(true);

Check failure on line 227 in packages/artifact/__tests__/download-artifact.test.ts

View workflow job for this annotation

GitHub Actions / Build (ubuntu-latest)

Replace `fs.existsSync(path.join(fixtures.workspaceDir,·'y/etc/hosts'))).toBe(true);` with `⏎········fs.existsSync(path.join(fixtures.workspaceDir,·'y/etc/hosts'))⏎······).toBe(true)`

Check failure on line 227 in packages/artifact/__tests__/download-artifact.test.ts

View workflow job for this annotation

GitHub Actions / Build (ubuntu-latest)

Extra semicolon

Check failure on line 227 in packages/artifact/__tests__/download-artifact.test.ts

View workflow job for this annotation

GitHub Actions / Build (macos-latest)

Replace `fs.existsSync(path.join(fixtures.workspaceDir,·'y/etc/hosts'))).toBe(true);` with `⏎········fs.existsSync(path.join(fixtures.workspaceDir,·'y/etc/hosts'))⏎······).toBe(true)`

Check failure on line 227 in packages/artifact/__tests__/download-artifact.test.ts

View workflow job for this annotation

GitHub Actions / Build (macos-latest)

Extra semicolon

Check failure on line 227 in packages/artifact/__tests__/download-artifact.test.ts

View workflow job for this annotation

GitHub Actions / Build (windows-latest)

Replace `fs.existsSync(path.join(fixtures.workspaceDir,·'y/etc/hosts'))).toBe(true);` with `␍⏎········fs.existsSync(path.join(fixtures.workspaceDir,·'y/etc/hosts'))␍⏎······).toBe(true)`

Check failure on line 227 in packages/artifact/__tests__/download-artifact.test.ts

View workflow job for this annotation

GitHub Actions / Build (windows-latest)

Extra semicolon

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.

52 changes: 2 additions & 50 deletions packages/artifact/src/internal/download/download-artifact.ts
Original file line number Diff line number Diff line change
@@ -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', () => {

Check failure on line 93 in packages/artifact/src/internal/download/download-artifact.ts

View workflow job for this annotation

GitHub Actions / Build (ubuntu-latest)

Delete `·`

Check failure on line 93 in packages/artifact/src/internal/download/download-artifact.ts

View workflow job for this annotation

GitHub Actions / Build (macos-latest)

Delete `·`

Check failure on line 93 in packages/artifact/src/internal/download/download-artifact.ts

View workflow job for this annotation

GitHub Actions / Build (windows-latest)

Delete `·`
clearTimeout(timer)
resolve()
})
Expand Down

0 comments on commit 476276b

Please sign in to comment.