-
Notifications
You must be signed in to change notification settings - Fork 0
Description
The extra-args input in .github/actions/setup-python-env/action.yml (line 42) uses a denylist to block specific dangerous uv sync flags, but this approach is insufficient — many other flags that can alter dependency resolution semantics remain unblocked.
Unblocked flags that could be dangerous:
--python— could redirect to a different Python interpreter--no-build-isolation— disables build isolation, allowing unbounded environment access--constraint/--override-requires-python— alter dependency resolution--no-sources— skips workspace packages
Recommendation: Replace the denylist with an allowlist of explicitly supported flags. For example:
allowed_args=(
"--all-packages"
"--prerelease=if-necessary-or-explicit"
"-U"
"--upgrade"
)
for arg in $EXTRA_ARGS; do
allowed=false
for a in "${allowed_args[@]}"; do [[ "$arg" == "$a" ]] && allowed=true && break; done
if [[ "$allowed" != "true" ]]; then
echo "::error::Unrecognized extra-args token: '$arg'." >&2; exit 1
fi
doneAlternatively, remove extra-args entirely and expose structured boolean/enum inputs for each supported flag.
File: .github/actions/setup-python-env/action.yml, line 42
Severity: Medium
Categories: Input Validation and Sanitization (Category 1) + CI/CD Security (Category 14)
Original review finding:
Category: Input Validation and Sanitization (Category 1) + CI/CD Security (Category 14)
Severity: MediumThe
extra-argsguard uses a denylist to block specific dangerous flags (--index-url,--extra-index-url,--trusted-host,--find-links), but many otheruv syncflags that can alter dependency resolution semantics are not blocked, for example:
--python— could redirect to a different Python interpreter--no-build-isolation— disables build isolation, allowing unbounded environment access--constraint/--override-requires-python— alter dependency resolution--no-sources— skips workspace packagesIf any caller ever forwards a dynamic/user-sourced string to
extra-args(despite the README warning), the denylist provides insufficient protection.Recommendation: Replace the denylist with an allowlist of explicitly supported flags.
Related PR: #51
Review comment: #51 (comment)
Generated by PR Review Comment — Create Issue for issue #51