-
Notifications
You must be signed in to change notification settings - Fork 870
fix(all): shell option usage on child_process.spawn calls #2708
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
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 issues found across 8 files
Prompt for AI agents (all 2 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/preview-server/scripts/build-preview-server.mts">
<violation number="1" location="packages/preview-server/scripts/build-preview-server.mts:5">
Spawning node_modules/.bin/next directly breaks on Windows because the .cmd shim is never selected; resolve to next.cmd (or use process.execPath) so the build still runs on win32.</violation>
</file>
<file name="packages/create-email/src/index.spec.ts">
<violation number="1" location="packages/create-email/src/index.spec.ts:44">
Type-check test now invokes the repo’s TypeScript binary instead of the generated project’s, so missing TypeScript dependencies in the starter will go undetected.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
fc9ac4c to
b4da70a
Compare
c96e840 to
0a4a3c9
Compare
|
@cubic-dev-ai review this PR |
@gabrielmfern I've started the AI code review. It'll take a few minutes to complete. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 8 files
Prompt for AI agents (all 1 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/create-email/tsconfig.json">
<violation number="1" location="packages/create-email/tsconfig.json:7">
TypeScript 5.8.3 (the version used by this package) does not support the `"node20"` module target, so the new compiler option makes `tsc` fail with TS6046.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
shell: trueruns the command you input using the shell from the system, but it doesn't sanitize the arguments you pass to the shell meaning it can be unsafe, so Node.js starts warning from version 24 onwards likeThis pull request simply replaces all of those usages with all the arguments as a raw string, or using nypm to install dependencies and run scripts, which also makes our code cleaner.
Summary by cubic
Updated child_process.spawn usage to avoid unsafe shell argument handling and reduce Node v24 warnings by switching to nypm helpers and safer command execution. Preview server scripts now use single command strings with shell and import.meta.dirname for reliable paths.
Written for commit 032309b. Summary will update automatically on new commits.