-
Notifications
You must be signed in to change notification settings - Fork 56
Upgrade node from v18 to v24 #1106
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
base: main
Are you sure you want to change the base?
Conversation
|
Plz update the lint_driver and trunk.yaml node versions as well, i.e. https://github.com/search?q=repo%3Atrunk-io%2Fplugins%20node%40&type=code |
|
|
|
|
|
| encoding: "utf8" as const, | ||
| windowsHide: options.windowsHide, | ||
| }; | ||
| const printConfig = execSync([executable, ...args].join(" "), execOptions); |
Check warning
Code scanning / CodeQL
Shell command built from environment values Medium test
absolute path
This shell command depends on an uncontrolled
absolute path
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 20 hours ago
In general, to fix this class of issue you should avoid constructing a single shell command string that mixes executable paths and arguments; instead, pass the executable and arguments as separate parameters to execFileSync/execFile (or the platform equivalent). This prevents environment-controlled values (paths, args) from being interpreted by the shell.
For this concrete code, the best fix is to change getFullTrunkConfig so it no longer calls execSync with [executable, ...args].join(" "). We already have buildExecArgs returning the executable, arguments array, and options suitable for execFileSync. So in getFullTrunkConfig we can:
- Keep using
buildExecArgs(["config", "print"])to get[executable, args, options]. - Replace the
execSynccall withexecFileSync(executable, args, execOptions), whereexecOptionsis built as it is now (without specifyingshell: true). - This removes the need to join into a single string, so no shell interpretation occurs, and any spaces or special characters in
ARGS.cliPathor args are handled safely.
The only file needing modification is tests/driver/driver.ts, within the getFullTrunkConfig method. No new imports or helpers are required because execFileSync is already imported at the top of the file.
-
Copy modified line R313
| @@ -310,7 +310,7 @@ | ||
| encoding: "utf8" as const, | ||
| windowsHide: options.windowsHide, | ||
| }; | ||
| const printConfig = execSync([executable, ...args].join(" "), execOptions); | ||
| const printConfig = execFileSync(executable, args, execOptions); | ||
| return YAML.parse(printConfig.replaceAll("\r\n", "\n")); | ||
| }; | ||
|
|
No description provided.