mirror of
https://github.com/game-ci/unity-builder.git
synced 2026-09-29 12:07:05 -07:00
docs: explain why exec.exec(cliPath, args, ...) isn't shell-injectable
CodeQL flagged this line (js/command-line-injection, critical) since args ultimately derives from Action inputs, and its static analysis can't see through @actions/exec's internals to confirm safety. Verified this is a genuine false positive by reading the actual dependency source: @actions/exec's toolrunner.js passes args straight to child_process.spawn(fileName, args, options) - node_modules/@actions/exec/lib/toolrunner.js line 413 - never a shell string, never shell-parsed. args is already an array of discrete argv entries (from buildCliArgs), matching CodeQL's own stated recommendation for the safe pattern here (arguments as an array, not a concatenated string) exactly. Added a documented comment explaining this at the flagged line, since I can't verify without seeing a re-run whether this repo's CodeQL setup honors inline suppression comments - if the check doesn't clear on the next analysis, the alert likely needs dismissing via the Security tab instead (a maintainer action, not something achievable from a commit). No functional change - comment only.
This commit is contained in:
+9
@@ -477,6 +477,15 @@ async function run() {
|
||||
const args = (0, build_args_1.buildCliArgs)({
|
||||
getInput: (name) => (name === 'projectPath' ? projectPath : core.getInput(name)),
|
||||
});
|
||||
// codeql[js/command-line-injection] - args is an array of discrete argv
|
||||
// entries, not a concatenated shell string, and @actions/exec's
|
||||
// toolrunner.js passes it straight to child_process.spawn(fileName, args,
|
||||
// options) (verified by reading node_modules/@actions/exec/lib/toolrunner.js)
|
||||
// - no shell is ever invoked to (mis)parse it, so classic shell/command
|
||||
// injection via metacharacters isn't reachable here. CodeQL's static
|
||||
// analysis can't see through @actions/exec's internals to confirm that,
|
||||
// which is why it still flags this generic "user input reaches an
|
||||
// exec-family call" pattern.
|
||||
const exitCode = await exec.exec(cliPath, args, { ignoreReturnCode: true });
|
||||
// Matches the original action's engineExitCode output: 0 on success,
|
||||
// otherwise the exit code of whichever step (activation or build) - or,
|
||||
|
||||
+1
-1
File diff suppressed because one or more lines are too long
@@ -40,6 +40,15 @@ export async function run() {
|
||||
getInput: (name) => (name === 'projectPath' ? projectPath : core.getInput(name)),
|
||||
});
|
||||
|
||||
// codeql[js/command-line-injection] - args is an array of discrete argv
|
||||
// entries, not a concatenated shell string, and @actions/exec's
|
||||
// toolrunner.js passes it straight to child_process.spawn(fileName, args,
|
||||
// options) (verified by reading node_modules/@actions/exec/lib/toolrunner.js)
|
||||
// - no shell is ever invoked to (mis)parse it, so classic shell/command
|
||||
// injection via metacharacters isn't reachable here. CodeQL's static
|
||||
// analysis can't see through @actions/exec's internals to confirm that,
|
||||
// which is why it still flags this generic "user input reaches an
|
||||
// exec-family call" pattern.
|
||||
const exitCode = await exec.exec(cliPath, args, { ignoreReturnCode: true });
|
||||
|
||||
// Matches the original action's engineExitCode output: 0 on success,
|
||||
|
||||
Reference in New Issue
Block a user