Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix custom deploy zip file size limitation #2133

Merged
merged 5 commits into from
Dec 4, 2024

Conversation

luismulinari
Copy link
Contributor

@luismulinari luismulinari commented Dec 2, 2024

Description

This pull request replaces the adm-zip library with node-stream-zip to address a critical limitation: adm-zip does not support processing files larger than 2GB (cthackers/adm-zip#334). As a result, our custom deploy validation fails when handling large zip files. The new library, node-stream-zip, provides robust support for large archives and offers a more efficient, stream-based approach to working with zip files.

Pull request checklist

New release checklist

Steps to Test

Outline the steps to test and verify the PR here.

Example:

  1. Check out PR.
  2. Run npm run build
  3. Run node ./dist/bin/vip-app-deploy-validate.js /tmp/valid-zip.zip

Currently, adm-zip does not support files bigger than 2GB (cthackers/adm-zip#334).

To fix it, the ZIP library was replaced by node-stream-zip, which supports files bigger than 2GB.
Copy link
Contributor

github-actions bot commented Dec 2, 2024

Dependency Review

The following issues were found:
  • ✅ 0 vulnerable package(s)
  • ✅ 0 package(s) with incompatible licenses
  • ✅ 0 package(s) with invalid SPDX license definitions
  • ✅ 0 package(s) with unknown licenses.
  • ⚠️ 1 packages with OpenSSF Scorecard issues.
See the Details below.

OpenSSF Scorecard

PackageVersionScoreDetails
npm/node-stream-zip 1.15.0 ⚠️ 2.6
Details
CheckScoreReason
Maintained⚠️ 00 commit(s) and 0 issue activity found in the last 90 days -- score normalized to 0
Code-Review⚠️ 0Found 2/27 approved changesets -- score normalized to 0
Packaging⚠️ -1packaging workflow not detected
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Token-Permissions⚠️ 0detected GitHub workflow tokens with excessive permissions
Binary-Artifacts🟢 9binaries present in source code
Pinned-Dependencies🟢 3dependency not pinned by hash detected -- score normalized to 3
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Security-Policy⚠️ 0security policy file not detected
Fuzzing⚠️ 0project is not fuzzed
Signed-Releases⚠️ -1no releases found
License🟢 9license file detected
Branch-Protection⚠️ 0branch protection not enabled on development/release branches
SAST⚠️ 0SAST tool is not run on all commits -- score normalized to 0
Vulnerabilities⚠️ 023 existing vulnerabilities detected

Scanned Files

  • package.json

@luismulinari luismulinari changed the title Replce adm-zip by node-stream-zip Fix custom deploy zip file size limitation Dec 2, 2024
@luismulinari luismulinari requested a review from a team December 2, 2024 19:55
@luismulinari luismulinari force-pushed the replace-admzip-by-node-stream-zip branch from 4ff57be to 7a590e1 Compare December 2, 2024 19:59
@luismulinari luismulinari force-pushed the replace-admzip-by-node-stream-zip branch from 7a590e1 to 6ccf597 Compare December 2, 2024 20:01
@rebeccahum
Copy link
Contributor

Tested and works good, we should look at why the tests are failing though.

@luismulinari luismulinari force-pushed the replace-admzip-by-node-stream-zip branch from c207eb8 to e97a9e1 Compare December 2, 2024 22:56
@luismulinari luismulinari force-pushed the replace-admzip-by-node-stream-zip branch from 89f8d8d to 2a12396 Compare December 3, 2024 13:19
Copy link

sonarcloud bot commented Dec 3, 2024

@luismulinari luismulinari merged commit 4f30fd5 into trunk Dec 4, 2024
17 checks passed
@luismulinari luismulinari deleted the replace-admzip-by-node-stream-zip branch December 4, 2024 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants