use extract-zip and tar libraries to extract archives#10414
Conversation
Summary of ChangesHello @jakemac53, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the archive extraction mechanism within the CLI, moving away from relying on external system utilities like Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request is a great step towards improving cross-platform stability by replacing system-level tar and unzip commands with JavaScript libraries. The implementation is clean and the new tests are thorough. However, I've identified a critical security vulnerability related to the use of the unzipper library, which does not protect against "zip bomb" attacks. This could expose the CLI to denial-of-service attacks when processing malicious archives.
|
hmmm, looks like unzipper has made some unfortunate decisions that cause errors for our builds ZJONSSON/node-unzipper#330 |
|
Size Change: +143 kB (+0.81%) Total Size: 17.7 MB
ℹ️ View Unchanged
|
|
@jakemac53 if you updated package.json we need to see an equivalent package lock update or we will be out of sync. |
Co-authored-by: christine betts <chrstn@uw.edu>
…10414) Co-authored-by: christine betts <chrstn@uw.edu>
TLDR
This should result in more stable and consistent archive extraction across operating systems and users machines, as opposed to assuming that tools like
tarandunzipare installed.The current approach has been broken a couple times recently and I am still not even confident the current implementation actually works on windows reliably, especially across windows versions.
Dive Deeper
This does add a fair number of package dependencies so we should discuss whether that is an issue.
Reviewer Test Plan
Install an extension which has github releases.
Testing Matrix
Linked issues / bugs