FOUR-28753: [TCE] TCE Military V1 - Phase 1: Update Script Executor for Intake #8672
Conversation
…ild arguments for docker
FOUR-28393 BASES Improve the script executor building
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on January 21
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| } | ||
|
|
||
| return []; | ||
| } |
There was a problem hiding this comment.
Shell command injection via unsanitized build-args input
High Severity
The getBuildArgs() method constructs shell command arguments by directly concatenating user-supplied input from the --build-args option without escaping. The value is split by comma and each part is appended as --build-arg $arg to the Docker command, which is then passed to system() or proc_open(). An attacker could inject arbitrary shell commands (e.g., --build-args="FOO=bar; malicious-command"). The codebase uses escapeshellarg() elsewhere for this purpose, such as in ScriptRunners/Base.php and TenantsCreate.php.
🔬 Verification Test
Why verification test was not possible: This is a shell command injection vulnerability that would require actually running the artisan command with malicious input against a Docker environment. Running such a test could cause unintended system changes. However, the vulnerability is evident from code inspection: the $arg variable from user input is directly concatenated into a shell command string ('--build-arg ' . $arg) without any call to escapeshellarg(), and this string is ultimately passed to system() or proc_open(). The pattern used elsewhere in the codebase (e.g., escapeshellarg() in Base.php lines 88, 181, 190) demonstrates the expected sanitization that is missing here.
Additional Locations (1)
pmPaulis
changed the title
|
|
||
| foreach ($buildArgs as $buildArg) { | ||
| $command .= ' ' . $buildArg; | ||
| } |
There was a problem hiding this comment.
Build args appended after docker build context path
Medium Severity
The --build-arg options from getBuildArgs() are appended after the build context path {$packagePath} in the docker build command. Docker build syntax requires all options to precede the PATH argument (docker build [OPTIONS] PATH). The resulting command places --build-arg values after the context path, which means the new --build-args feature will not work correctly as Docker expects the build context to be the final argument.
| exec( | ||
| $docker . ' run -d --name ' . $instanceName . '_nayra ' | ||
| $docker . ' run -d ' | ||
| . ($this->getNayraPort() !== 8080 ? '-p ' . $this->getNayraPort() . ':8080 ' : '') |
There was a problem hiding this comment.
Port mapping missing in executor build startup path
Medium Severity
The commit adds port mapping logic to bringUpNayra (the instance method) but not to bringUpNayraExecutor (the static method called during build at line 176 of BuildScriptExecutors.php). When a Nayra executor is built, bringUpNayraExecutor starts the container without the -p port mapping flag. Later, getNayraInstanceUrl() constructs a URL using the configured port, but the container wasn't started with that port mapped, causing connection failures when a custom nayra_port is configured.
|
Issue & Reproduction Steps
TCE Military V1 - Phase 1: Update Script Executor for Intake
Solution
How to Test
Describe how to test that this solution works.
Related Tickets & Packages
Code Review Checklist
Note
Introduces flexibility in executor builds and Nayra runtime configuration.
processmaker:build-script-executorto accept--build-argsand appends them todocker build(e.g.,--build-arg key=value).config('app.nayra_port')(defaults to8080); removes static port usage and usesgetNayraPort()in URLs and container startup.-e PORT=<port>on host network or-p <port>:8080on other networks, applied when port != 8080.nayra_porttoconfig/app.phpand uses it throughout Nayra Docker handling.Written by Cursor Bugbot for commit bb21d76. This will update automatically on new commits. Configure here.