gemini-code-assist[bot] commented on code in PR #35733: URL: https://github.com/apache/beam/pull/35733#discussion_r2241457073
########## sdks/python/scripts/run_pytest.sh: ########## @@ -62,16 +81,60 @@ if [[ -n $user_marker ]]; then marker_for_sequential_tests="($user_marker) and ($marker_for_sequential_tests)" fi +# Parse posargs to separate pytest options from test paths. +options="" +test_paths="" + +# Safely split the posargs string into individual arguments. +eval "set -- $posargs" Review Comment: I understand your concerns regarding increased complexity and the review process in CI environments. However, the use of `eval` with user-controlled input like `$posargs` inherently introduces a critical command injection vulnerability. This is a fundamental security flaw that allows arbitrary code execution, regardless of whether the script is run in a CI environment or how frequently it's reviewed. Even in a controlled CI setting, a successful exploit could lead to severe consequences, such as data exfiltration, system compromise, or disruption of the build process. While parsing shell arguments safely can be complex, the security implications of `eval` on untrusted input are too significant to ignore. Prioritizing security often necessitates refactoring for safer alternatives, even if it introduces some additional complexity. I maintain that this is a `critical` issue that should be addressed to prevent potential security breaches. -- 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]
