diff --git a/.github/workflows/download.yml b/.github/workflows/download.yml index 50390cec..3091ac01 100644 --- a/.github/workflows/download.yml +++ b/.github/workflows/download.yml @@ -121,6 +121,20 @@ jobs: ref: ${{github.event.pull_request.head.sha || github.sha}} - name: Test run: cat artifact/sha | grep $GITHUB_SHA + download-unarchived: + runs-on: ubuntu-latest + needs: wait + steps: + - name: Checkout + uses: actions/checkout@v6 + - name: Download + uses: ./ + with: + workflow: upload.yml + name: sha + path: artifact-unarchived + - name: Test + run: cat artifact-unarchived/sha | grep $GITHUB_SHA download-multiple: runs-on: ubuntu-latest needs: wait @@ -136,6 +150,7 @@ jobs: cat artifact/sha | grep $GITHUB_SHA cat artifact1/sha1 | grep $GITHUB_SHA cat artifact2/sha2 | grep $GITHUB_SHA + cat sha/sha | grep $GITHUB_SHA download-regexp: runs-on: ubuntu-latest needs: wait diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 00000000..ce3d745b --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,22 @@ +name: Test + +on: + push: + branches: + - master + pull_request: + +jobs: + test: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v6 + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: '20' + - name: Install dependencies + run: npm install + - name: Run tests + run: npm test diff --git a/.github/workflows/upload.yml b/.github/workflows/upload.yml index b5a242d7..85479383 100644 --- a/.github/workflows/upload.yml +++ b/.github/workflows/upload.yml @@ -22,6 +22,16 @@ jobs: with: name: artifact path: artifact + upload-unarchived: + runs-on: ubuntu-latest + steps: + - name: Dump + run: echo $GITHUB_SHA > sha + - name: Upload + uses: actions/upload-artifact@v7 + with: + path: sha + archive: false upload-multiple: runs-on: ubuntu-latest steps: diff --git a/main.js b/main.js index a9a385c5..2cfaebe5 100644 --- a/main.js +++ b/main.js @@ -6,6 +6,14 @@ import AdmZip from 'adm-zip' import { filesize } from 'filesize' import pathname from 'node:path' import fs from 'node:fs' +import { fileURLToPath } from 'node:url' + +export function isZipContentType(contentType) { + const mimeType = (contentType || '').split(';')[0].trim().toLowerCase() + return mimeType === 'application/zip' || + mimeType === 'application/x-zip-compressed' || + mimeType === 'application/zip-compressed' +} async function downloadAction(name, path) { const artifactClient = artifact.create() @@ -262,15 +270,16 @@ async function main() { const size = filesize(artifact.size_in_bytes, { base: 10 }) - core.info(`==> Downloading: ${artifact.name}.zip (${size})`) + core.info(`==> Downloading: ${artifact.name} (${size})`) - let zip + let downloadResponse try { - zip = await client.rest.actions.downloadArtifact({ + downloadResponse = await client.rest.actions.downloadArtifact({ owner: owner, repo: repo, artifact_id: artifact.id, archive_format: "zip", + request: { redirect: 'manual' }, }) } catch (error) { if (error.message.startsWith("Artifact has expired")) { @@ -280,9 +289,28 @@ async function main() { } } + const blobUrl = downloadResponse.headers.location + core.debug(`Download URL: ${blobUrl}`) + + const response = await fetch(blobUrl) + + if (!response.ok) { + throw new Error(`Failed to download artifact: ${response.statusText}`) + } + + const contentType = response.headers.get('content-type') || '' + const urlPath = new URL(blobUrl).pathname.toLowerCase() + const urlIndicatesZip = urlPath.endsWith('.zip') + const isZipFile = isZipContentType(contentType) || urlIndicatesZip + + core.debug(`Content-Type: ${contentType}, URL path: ${urlPath}, Detected as zip: ${isZipFile}`) + + const buffer = Buffer.from(await response.arrayBuffer()) + if (skipUnpack) { fs.mkdirSync(path, { recursive: true }) - fs.writeFileSync(`${pathname.join(path, artifact.name)}.zip`, Buffer.from(zip.data), 'binary') + const ext = isZipFile ? '.zip' : '' + fs.writeFileSync(`${pathname.join(path, artifact.name)}${ext}`, buffer, 'binary') continue } @@ -290,23 +318,32 @@ async function main() { fs.mkdirSync(dir, { recursive: true }) - core.startGroup(`==> Extracting: ${artifact.name}.zip`) - if (useUnzip) { - const zipPath = `${pathname.join(dir, artifact.name)}.zip` - fs.writeFileSync(zipPath, Buffer.from(zip.data), 'binary') - await exec.exec("unzip", [zipPath, "-d", dir]) - fs.rmSync(zipPath) + if (!isZipFile) { + const contentDisposition = response.headers.get('content-disposition') || '' + const cdMatch = contentDisposition.match(/filename[^;=\n]*=((['"]).*?\2|[^;\n]*)/) + const rawFilename = cdMatch ? cdMatch[1].replace(/^['"]|['"]$/g, '').trim() : artifact.name + const filename = pathname.basename(rawFilename) || artifact.name + core.info(`==> Writing direct file: ${filename}`) + fs.writeFileSync(pathname.join(dir, filename), buffer, 'binary') } else { - const adm = new AdmZip(Buffer.from(zip.data)) - adm.getEntries().forEach((entry) => { - const action = entry.isDirectory ? "creating" : "inflating" - const filepath = pathname.join(dir, entry.entryName) - - core.info(` ${action}: ${filepath}`) - }) - adm.extractAllTo(dir, true) + core.startGroup(`==> Extracting: ${artifact.name}.zip`) + if (useUnzip) { + const zipPath = `${pathname.join(dir, artifact.name)}.zip` + fs.writeFileSync(zipPath, buffer, 'binary') + await exec.exec("unzip", [zipPath, "-d", dir]) + fs.rmSync(zipPath) + } else { + const adm = new AdmZip(buffer) + adm.getEntries().forEach((entry) => { + const action = entry.isDirectory ? "creating" : "inflating" + const filepath = pathname.join(dir, entry.entryName) + + core.info(` ${action}: ${filepath}`) + }) + adm.extractAllTo(dir, true) + } + core.endGroup() } - core.endGroup() } } catch (error) { core.setOutput("found_artifact", false) @@ -332,4 +369,6 @@ async function main() { } } -main() +if (process.argv[1] === fileURLToPath(import.meta.url)) { + main() +} diff --git a/main.test.js b/main.test.js new file mode 100644 index 00000000..48a50b78 --- /dev/null +++ b/main.test.js @@ -0,0 +1,183 @@ +import { test, describe, beforeEach, afterEach } from 'node:test' +import assert from 'node:assert/strict' +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import AdmZip from 'adm-zip' + +// main.js is guarded by import.meta.url so importing it does not call main(). +const { isZipContentType } = await import('./main.js') + +// --------------------------------------------------------------------------- +// isZipContentType – unit tests +// --------------------------------------------------------------------------- + +describe('isZipContentType', () => { + test('returns true for application/zip', () => { + assert.equal(isZipContentType('application/zip'), true) + }) + + test('returns true for application/x-zip-compressed', () => { + assert.equal(isZipContentType('application/x-zip-compressed'), true) + }) + + test('returns true for application/zip-compressed', () => { + assert.equal(isZipContentType('application/zip-compressed'), true) + }) + + test('returns true for application/zip with extra parameters', () => { + assert.equal(isZipContentType('application/zip; charset=utf-8'), true) + }) + + test('returns true regardless of case', () => { + assert.equal(isZipContentType('Application/Zip'), true) + assert.equal(isZipContentType('APPLICATION/ZIP'), true) + }) + + // archive: false uploads use application/octet-stream or similar + test('returns false for application/octet-stream (archive: false direct upload)', () => { + assert.equal(isZipContentType('application/octet-stream'), false) + }) + + test('returns false for text/plain', () => { + assert.equal(isZipContentType('text/plain'), false) + }) + + test('returns false for image/png', () => { + assert.equal(isZipContentType('image/png'), false) + }) + + test('returns false for empty string', () => { + assert.equal(isZipContentType(''), false) + }) + + test('returns false for null', () => { + assert.equal(isZipContentType(null), false) + }) + + test('returns false for undefined', () => { + assert.equal(isZipContentType(undefined), false) + }) +}) + +// --------------------------------------------------------------------------- +// Download behaviour – integration tests using a real temp directory +// --------------------------------------------------------------------------- + +describe('artifact download behaviour', () => { + let tmpDir + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'artifact-test-')) + }) + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }) + }) + + test('non-ZIP artifact (archive: false) is written as a plain file', () => { + // Simulate what main.js does when isZipFile is false: + // fs.writeFileSync(pathname.join(dir, filename), buffer, 'binary') + const artifactName = 'my-binary' + const content = 'raw file content' + const buffer = Buffer.from(content) + + // isZipContentType must return false for a direct-upload content type + assert.equal(isZipContentType('application/octet-stream'), false) + + const outputPath = path.join(tmpDir, artifactName) + fs.writeFileSync(outputPath, buffer, 'binary') + + assert.ok(fs.existsSync(outputPath), 'output file should exist') + assert.equal(fs.readFileSync(outputPath, 'utf8'), content) + }) + + test('ZIP artifact is extracted to the target directory', () => { + // Build a minimal in-memory zip containing one entry + const adm = new AdmZip() + adm.addFile('hello.txt', Buffer.from('hello from zip')) + const zipBuffer = adm.toBuffer() + + // isZipContentType must return true for application/zip + assert.equal(isZipContentType('application/zip'), true) + + // Simulate main.js ZIP extraction path (adm-zip branch) + const adm2 = new AdmZip(zipBuffer) + adm2.extractAllTo(tmpDir, true) + + const extracted = path.join(tmpDir, 'hello.txt') + assert.ok(fs.existsSync(extracted), 'extracted file should exist') + assert.equal(fs.readFileSync(extracted, 'utf8'), 'hello from zip') + }) + + test('skip_unpack with non-ZIP writes file without .zip extension', () => { + const artifactName = 'my-report' + const buffer = Buffer.from('report data') + const isZipFile = isZipContentType('application/octet-stream') // false + + const ext = isZipFile ? '.zip' : '' + const outputPath = path.join(tmpDir, `${artifactName}${ext}`) + fs.writeFileSync(outputPath, buffer, 'binary') + + // Should not have .zip extension + assert.ok(fs.existsSync(path.join(tmpDir, 'my-report')), 'file without extension should exist') + assert.ok(!fs.existsSync(path.join(tmpDir, 'my-report.zip')), 'file with .zip extension should not exist') + }) + + test('skip_unpack with ZIP writes file with .zip extension', () => { + const adm = new AdmZip() + adm.addFile('data.txt', Buffer.from('data')) + const artifactName = 'my-archive' + const buffer = adm.toBuffer() + const isZipFile = isZipContentType('application/zip') // true + + const ext = isZipFile ? '.zip' : '' + const outputPath = path.join(tmpDir, `${artifactName}${ext}`) + fs.writeFileSync(outputPath, buffer, 'binary') + + assert.ok(fs.existsSync(path.join(tmpDir, 'my-archive.zip')), 'file with .zip extension should exist') + assert.ok(!fs.existsSync(path.join(tmpDir, 'my-archive')), 'file without extension should not exist') + }) + + // ------------------------------------------------------------------------- + // URL-based ZIP detection + // ------------------------------------------------------------------------- + test('URL path ending with .zip is detected as ZIP', () => { + const blobUrl = 'https://storage.example.com/artifact-sha.zip?sig=abc' + const urlPath = new URL(blobUrl).pathname.toLowerCase() + assert.ok(urlPath.endsWith('.zip'), 'URL pathname should end with .zip') + }) + + test('URL path not ending with .zip is not detected as ZIP via URL', () => { + const blobUrl = 'https://storage.example.com/artifact-sha?sig=abc' + const urlPath = new URL(blobUrl).pathname.toLowerCase() + assert.ok(!urlPath.endsWith('.zip'), 'URL pathname should not end with .zip') + }) + + // ------------------------------------------------------------------------- + // Content-Disposition filename extraction + // ------------------------------------------------------------------------- + test('Content-Disposition filename is used when present', () => { + // Simulate the filename extraction logic from main.js + function extractFilename(contentDisposition, fallback) { + const cdMatch = contentDisposition.match(/filename[^;=\n]*=((['"]).*?\2|[^;\n]*)/) + const rawFilename = cdMatch ? cdMatch[1].replace(/^['"]|['"]$/g, '').trim() : fallback + return path.basename(rawFilename) || fallback + } + + assert.equal(extractFilename('attachment; filename="sha"', 'artifact-name'), 'sha') + assert.equal(extractFilename('attachment; filename=sha', 'artifact-name'), 'sha') + assert.equal(extractFilename('', 'artifact-name'), 'artifact-name') + }) + + test('path.basename sanitizes path traversal in artifact name', () => { + const dangerousName = '../../../etc/passwd' + const safe = path.basename(dangerousName) + assert.equal(safe, 'passwd') + // Ensure writing to a tmpDir using the sanitized name stays within tmpDir + const outputPath = path.join(tmpDir, safe) + fs.writeFileSync(outputPath, 'data', 'utf8') + assert.ok(fs.existsSync(outputPath)) + }) +}) + diff --git a/package.json b/package.json index 90d24314..d0a56a61 100644 --- a/package.json +++ b/package.json @@ -2,6 +2,9 @@ "name": "action-download-artifact", "type": "module", "main": "main.js", + "scripts": { + "test": "node --test main.test.js" + }, "dependencies": { "@actions/artifact": "^6.2.0", "@actions/core": "^3.0.0",