[XPU][CI]add xpu env vars in CI scripts #22946
Conversation
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels. Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add 🚀 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds the HF_TOKEN and ZE_AFFINITY_MASK environment variables to the Docker container in the XPU CI test script. This change is intended to ensure the tests have the necessary environment to run correctly, particularly for accessing gated models and controlling GPU affinity. The change also includes a debug statement to print the value of ZE_AFFINITY_MASK. While the overall change is beneficial, the new debug statement introduces a critical command injection vulnerability by not quoting the environment variable. My review includes a specific comment and a code suggestion to address this security risk.
There was a problem hiding this comment.
The ZE_AFFINITY_MASK variable is unquoted in the echo command. This is a command injection vulnerability. If an attacker can control the value of ZE_AFFINITY_MASK, they could execute arbitrary commands inside the container. For example, if ZE_AFFINITY_MASK is set to $(reboot), the container would attempt to reboot. Always quote variables that might contain user-controllable or unexpected data to prevent shell expansion and command substitution.1
| echo $ZE_AFFINITY_MASK | |
| echo "$ZE_AFFINITY_MASK" |
Style Guide References
Footnotes
-
Unquoted variables in shell scripts can lead to command injection and other unexpected behavior due to word splitting, pathname expansion, and command substitution. It is a security best practice to always double-quote variables unless you specifically need the shell to perform these expansions. ↩
jikunshang
force-pushed
the
kunshang/ci_env
branch
2 times, most recently
from
37b34c6 to
ba9d202
Compare
github-actions
bot
added
the
ready
Signed-off-by: Kunshang Ji <kunshang.ji@intel.com>
Signed-off-by: Kunshang Ji <kunshang.ji@intel.com>
Signed-off-by: Kunshang Ji <kunshang.ji@intel.com>
Signed-off-by: Kunshang Ji <kunshang.ji@intel.com>
Signed-off-by: Kunshang Ji <kunshang.ji@intel.com>
Signed-off-by: Kunshang Ji <kunshang.ji@intel.com>
Signed-off-by: Kunshang Ji <kunshang.ji@intel.com> Signed-off-by: Duncan Moss <djm.moss@gmail.com>
Signed-off-by: Kunshang Ji <kunshang.ji@intel.com>
Signed-off-by: Kunshang Ji <kunshang.ji@intel.com> Signed-off-by: Xiao Yu <xiao.yu@amd.com>
Signed-off-by: Kunshang Ji <kunshang.ji@intel.com>
Signed-off-by: Kunshang Ji <kunshang.ji@intel.com>
2 participants
Purpose
some env vars are not passed to container in ci scirpts. so add them
Test Plan
CI test
Test Result
(Optional) Documentation Update
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.