fix(lint): secure and OS-agnostic lint scripts and Windows formatting fixes#25368
fix(lint): secure and OS-agnostic lint scripts and Windows formatting fixes#25368r-siddiq wants to merge 2 commits intogoogle-gemini:mainfrom
Conversation
Summary of ChangesHello, 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 improves the reliability and security of the project's linting infrastructure. By transitioning from shell-based command execution to safer Node.js process spawning, the changes resolve critical security concerns while simultaneously enabling seamless operation for contributors working on Windows environments. Additionally, minor configuration adjustments were made to clean up the development experience. 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. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request enhances the cross-platform compatibility of the project's linting scripts, with a focus on supporting Windows. It introduces a more robust runCommand function using spawnSync, adds a getGitFiles utility to avoid shell pipe issues, and refactors the execution logic for ESLint, Actionlint, Shellcheck, Yamllint, and Prettier to handle platform-specific differences. The .gitignore and .prettierignore files were also updated to include relevant exclusions. I have no feedback to provide.
Summary
This PR addresses the Windows environment linting failures by making the linter execution OS-agnostic, while simultaneously fixing a critical command injection vulnerability identified in PR #22177. It also includes minor workspace formatting fixes to improve the contributor experience on Windows.
Details
Here is the breakdown of each file change and why it was needed:
scripts/lint.js: RefactoredrunCommandand the individual linting functions (runShellcheck,runYamllint,runActionlint,runESLint,runPrettier). Replaced unsafe string concatenation andshell: truewithspawnSyncusing an array of arguments andshell: false. This ensures filenames containing shell metacharacters are treated as literals, completely mitigating the command injection vulnerability. We conditionally allowshell: trueonly fornpm.cmdandnpx.cmdon Windows to enable path resolution..prettierignore: AddedCONTRIBUTING.mdto the ignore list. Prettier was adding newlines to the file, which caused annoying formatting issues and Git diff noise for Windows contributors..gitignore: Addedpackages/core/src/sandbox/windows/*.exeto ignore the Windows sandbox helper binaries generated during thenpm run preflightbuild step. Without this, the generated binary showed up as an untracked file change in Git.Related Issues
Fixes #22149
Fixes #25365
How to Validate
npm run preflight..exebinaries are shown as untracked ingit statusafter the build.scripts/lint.jsto confirmspawnSyncis used safely with argument arrays.Pre-Merge Checklist