feat: Handle non-zipped artifacts from upload-artifact@v7 (archive: false)#1
feat: Handle non-zipped artifacts from upload-artifact@v7 (archive: false)#1Copilot wants to merge 7 commits into
Conversation
Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
c1c99ec to
12a2d6a
Compare
|
@copilot Add tests for the upload-artifact functionality with |
There was a problem hiding this comment.
Pull request overview
This PR updates the artifact download/extract flow to support upload-artifact@v7 artifacts uploaded with archive: false (raw/non-zip), avoiding failures from attempting ZIP extraction on non-zip downloads.
Changes:
- Switch artifact handling from unconditional ZIP extraction to detecting ZIP vs non-ZIP via
Content-Type. - For non-zip downloads, write the payload directly to disk; for zip downloads, preserve existing unzip/AdmZip extraction paths.
- Adjust
skip_unpackbehavior to avoid appending.zipfor non-zip artifacts.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| core.debug(`Download URL: ${downloadResponse.url}`) | ||
|
|
||
| const response = await fetch(downloadResponse.url) | ||
|
|
There was a problem hiding this comment.
downloadArtifact() is called without request: { redirect: 'manual' }, then the code fetches downloadResponse.url (the API endpoint) without any auth headers. For private repos this fetch will likely 401/404, and it may also trigger an unnecessary extra download. Consider mirroring @actions/artifact: request manual redirect, read downloadResponse.headers.location, and fetch the signed blob URL instead.
| const isZipFile = mimeType === 'application/zip' || | ||
| mimeType === 'application/x-zip-compressed' || | ||
| mimeType === 'application/zip-compressed' | ||
|
|
||
| core.debug(`Content-Type: ${contentType}, Detected as zip: ${isZipFile}`) |
There was a problem hiding this comment.
ZIP detection only checks Content-Type. Blob downloads can come back as application/octet-stream, and @actions/artifact also falls back to checking whether the final URL path ends with .zip (ignoring query params). Without that fallback, some ZIP artifacts may be misdetected as non-zip and written as a raw file.
| const isZipFile = mimeType === 'application/zip' || | |
| mimeType === 'application/x-zip-compressed' || | |
| mimeType === 'application/zip-compressed' | |
| core.debug(`Content-Type: ${contentType}, Detected as zip: ${isZipFile}`) | |
| const urlPath = new URL(downloadResponse.url).pathname.toLowerCase() | |
| const urlIndicatesZip = urlPath.endsWith('.zip') | |
| const isZipFile = mimeType === 'application/zip' || | |
| mimeType === 'application/x-zip-compressed' || | |
| mimeType === 'application/zip-compressed' || | |
| urlIndicatesZip | |
| core.debug(`Content-Type: ${contentType}, URL path: ${urlPath}, Detected as zip: ${isZipFile}`) |
| if (!isZipFile) { | ||
| core.info(`==> Writing direct file: ${artifact.name}`) | ||
| fs.writeFileSync(pathname.join(dir, artifact.name), buffer, 'binary') |
There was a problem hiding this comment.
For non-zip artifacts, the file is written as path.join(dir, artifact.name). artifact.name is user-controlled and may not match the original uploaded filename (for archive:false), and it can also contain path separators. Consider deriving the filename from the Content-Disposition header (and sanitizing with path.basename) similar to @actions/artifact, to preserve the correct name and prevent path traversal.
| const response = await fetch(downloadResponse.url) | ||
|
|
There was a problem hiding this comment.
The PR description mentions adding node-fetch, but the implementation uses the Node 20 global fetch and package.json does not include node-fetch. Please either update the description (and avoid adding an unused dependency) or explicitly import/use the intended dependency for consistency.
Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
Added tests in commit 1871e5c using Node.js's built-in Changes:
|
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
…rkflow tests Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
Done in commit 5136e6d.
|
…rchive:false Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
|
@copilot apply changes based on the comments in this thread |
…-Disposition filename Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
Applied all three review changes in commit 4e8c598:
Also added 4 new tests covering URL-based ZIP detection and Content-Disposition filename extraction/sanitization (19 total, all passing). |
upload-artifact@v7supports direct file uploads without zipping (archive: false). The action unconditionally treated all downloads as ZIPs, failing withADM-ZIP: Invalid or unsupported zip format. No END header found.Changes
main.js: Replace blind ZIP extraction with robust format-aware download handling:request: { redirect: 'manual' }withdownloadArtifact()and readdownloadResponse.headers.locationto obtain the signed blob URL directly — avoids a second unauthenticated request and works correctly on private reposContent-Typeusing the exportedisZipContentType()helper (application/zip,application/x-zip-compressed,application/zip-compressed) — same logic as@actions/artifact.zip, the artifact is treated as a ZIP regardless ofContent-Type— handles cases where ZIP artifacts are served asapplication/octet-streamContent-Dispositionresponse header, sanitized withpath.basename()to prevent path traversal; falls back toartifact.nameskip_unpack→ omit.zipextension for non-ZIP artifactsmain.test.js: Test file using Node.js built-innode:testrunner (no extra dependencies) with 19 tests covering:isZipContentTypeunit tests: all ZIP MIME variants, non-ZIP types (includingapplication/octet-streamused byarchive: false), edge cases (null,undefined, case-insensitivity, parameters)skip_unpackextension handling for both ZIP and non-ZIP.zippathname suffix)Content-Dispositionfilename extraction andpath.basenamepath-traversal sanitizationpackage.json: Added"test": "node --test main.test.js"script.github/workflows/test.yml: New CI workflow that runsnpm teston every push and pull request (uses Node 20, matching the action runtime).github/workflows/upload.yml: Addedupload-unarchivedjob that uploads a single file witharchive: false(omittingname:sinceupload-artifact@v7uses thepathvalue as the artifact name whenarchive: false).github/workflows/download.yml: Addeddownload-unarchivedjob that downloads the artifact namedsha(the path used at upload time) and verifies the raw file content; updateddownload-multipleto also verify the unarchived artifact atsha/shaFully backward-compatible: legacy zipped artifacts and v7 zipped artifacts continue to work unchanged.
Original prompt
Problem
With the upgrade to
upload-artifact@v7, GitHub Actions now supports direct file uploads without zipping (usingarchive: false). This action currently fails when trying to download these non-zipped artifacts with the error:The action assumes all downloaded artifacts are ZIP files and attempts to extract them with
adm-zip, which fails for direct file uploads.Root Cause
The current implementation in
main.jsalways treats downloaded artifacts as ZIP files:When artifacts are uploaded with
archive: falsein v7, they are stored as raw files, not ZIP archives.Solution
Implement the same detection approach used by the official
@actions/artifactpackage (from https://github.com/actions/toolkit/blob/main/packages/artifact/src/internal/download/download-artifact.ts):downloadArtifact()to access HTTP headersContent-Typeheader to determine if it's a ZIP fileImplementation Requirements
1. Add
node-fetchdependencyUpdate
package.jsonto include:{ "dependencies": { "@actions/artifact": "^6.2.0", "@actions/core": "^3.0.0", "@actions/github": "^9.0.0", "adm-zip": "^0.5.16", "filesize": "^11.0.13", "node-fetch": "^3.3.2" } }2. Update main.js download logic
Replace the artifact download and extraction section (around lines 267-310) with the following approach:
Benefits
Content-Typeheader (same as official actions)@actions/artifactimplementationTesting Considerations
The implementation should work with:
archive: true)archive: false)skip_unpackoptionuse_unzipoptionReferences
This pull request was created from Copilot chat.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.