Copilot commented on code in PR #2021:
URL: https://github.com/apache/auron/pull/2021#discussion_r2847755532
##########
auron-build.sh:
##########
@@ -158,6 +160,15 @@ while [[ $# -gt 0 ]]; do
exit 1
fi
;;
+ --threads)
+ if [[ -n "$2" && "$2" != -* ]]; then
+ THREADS="$2"
+ shift 2
+ else
+ echo "ERROR: --threads requires a value (e.g. 1, 4, 1C)" >&2
+ exit 1
+ fi
Review Comment:
`--threads` currently accepts any non-empty value that doesn’t start with
`-`, so invalid inputs (e.g. `0`, `abc`, values containing whitespace) won’t be
caught until Maven fails later. Consider validating against Maven’s supported
`-T` formats (e.g. positive integer or `<float>C`) and rejecting
whitespace/glob characters to keep behavior predictable (especially since
Docker mode later expands args unquoted).
##########
auron-build.sh:
##########
@@ -396,6 +407,15 @@ if [[ -n "$ICEBERG_VER" ]]; then
BUILD_ARGS+=("-Piceberg-$ICEBERG_VER")
fi
+# Configure Maven build threads:
+# - local builds default to Maven's single-threaded behavior
+# - docker builds default to -T8 unless overridden
+if [[ -n "$THREADS" ]]; then
+ BUILD_ARGS+=("-T$THREADS")
+elif [[ "$USE_DOCKER" == true ]]; then
+ BUILD_ARGS+=("-T8")
+fi
Review Comment:
The script still allows passing Maven args directly after options (usage
shows `<maven build options>`), so users can now set `-T` in two places:
`--threads` and a raw Maven `-T...` argument. That can lead to
duplicate/ambiguous `-T` flags on local builds; consider detecting this
conflict (and erroring or letting one clearly override) to avoid
hard-to-diagnose build behavior differences.
##########
auron-build.sh:
##########
@@ -396,6 +407,15 @@ if [[ -n "$ICEBERG_VER" ]]; then
BUILD_ARGS+=("-Piceberg-$ICEBERG_VER")
fi
+# Configure Maven build threads:
+# - local builds default to Maven's single-threaded behavior
+# - docker builds default to -T8 unless overridden
+if [[ -n "$THREADS" ]]; then
Review Comment:
The value from `--threads` is stored in `THREADS` and appended directly to
`BUILD_ARGS`, which is later exported as `AURON_BUILD_ARGS` for Docker builds.
In `dev/docker-build/docker-compose.yml`, the container runs `./build/mvn
$AURON_BUILD_ARGS` without quoting, so a malicious `--threads` value (for
example `1; rm -rf /auron`) will break out of the Maven argument list and
execute arbitrary shell commands inside the Docker container. To mitigate this,
strictly validate `THREADS` to the expected Maven `-T` format (e.g., digits
with optional `C`) and/or refactor the Docker command to pass Maven arguments
as a safely-quoted array instead of an unquoted string.
```suggestion
if [[ -n "$THREADS" ]]; then
# Validate THREADS to prevent command injection when passed through
Docker
# Maven -T accepts an integer, optionally followed by 'C' (e.g., 4, 8,
2C)
if [[ ! "$THREADS" =~ ^[0-9]+C?$ ]]; then
echo "ERROR: Invalid --threads value '$THREADS'. Expected digits
with optional 'C' (e.g., 4, 8, 2C)." >&2
exit 1
fi
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
