Conversation
Summary of ChangesHello @skeshive, 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 resolves an issue where the license generation script for the VS Code extension was incorrectly identifying dependency versions, leading to an inaccurate 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
|
|
Size Change: -2 B (0%) Total Size: 21.6 MB ℹ️ View Unchanged
|
There was a problem hiding this comment.
Code Review
This pull request addresses a bug in the license generation script, correcting how it resolves dependencies to prioritize workspace-local package.json and package-lock.json entries. However, a high-severity Path Traversal vulnerability was identified in the generate-notices.js script, which could allow an attacker with commit access to read arbitrary files from the build environment due to unsafe path construction using dependency names from package.json without proper sanitization. Additionally, an area for improvement was noted regarding the robustness of file existence checks, which could benefit from more specific and reliable error handling.
| const localDepPath = path.join( | ||
| packagePath, | ||
| 'node_modules', | ||
| depName, | ||
| 'package.json', | ||
| ); | ||
| const rootDepPath = path.join( | ||
| projectRoot, | ||
| 'node_modules', | ||
| depName, | ||
| 'package.json', | ||
| ); |
There was a problem hiding this comment.
The depName variable, sourced from package.json, is used directly in path.join to construct file paths. This introduces a Path Traversal vulnerability. A malicious actor with commit access could craft a dependency name in package.json (e.g., "../../../../etc/passwd") to read arbitrary files from the system running the script. The file contents would then be embedded in the generated NOTICES.txt file.
Remediation:
Sanitize depName before use. Ensure the variable contains only valid package name characters and does not contain directory traversal sequences like ../. You can achieve this by validating the name against a strict regular expression or by ensuring that the resolved path is located within the expected parent directory.
…generation for workspace dependencies (google-gemini#15004)
TLDR
Updated
packages/vscode-ide-companion/scripts/generate-notices.jsto correctly prioritize and resolve workspace-local dependencies (likeexpress@5.1.0) over root dependencies. RegeneratedNOTICES.txtto reflect the accurate dependency tree for the VS Code extension.Dive Deeper
The previous license generation script was incorrectly resolving dependencies by looking at the root
node_modulesfirst or defaulting to root entries inpackage-lock.json. This caused issues where the script would pick up versions of packages used in the root workspace (e.g.,express@4.21.2) instead of the versions actually bundled with the extension (e.g.,express@5.1.0).This PR fixes the issue by:
package.jsonin the workspace'snode_modulesbefore falling back to the root.collectDependenciesnow attempts to find package entries under the workspace's path inpackage-lock.json(e.g.,packages/vscode-ide-companion/node_modules/express) before checking the global scope.NOTICES.txthas been regenerated. It now correctly listsexpress@5.1.0and its specific transitive dependencies (including new ones likeis-promiseand excluding unused ones likemethods), ensuring compliance for the bundled extension.