Merge V1.83.0 into Develop#5313
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request primarily serves to advance the HPC Toolkit to version 1.83.0, incorporating a series of version bumps across its core components and Terraform modules. Beyond this foundational update, it refines the environment setup for machine learning workloads by upgrading NVIDIA software dependencies and addresses a common issue with container imports in non-interactive shells. Additionally, network configurations in several GKE examples have been adjusted, and Cloud Build test parameters have been updated to enhance reliability and specificity. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request merges version 1.83.0 into the develop branch, primarily consisting of version bumps across various module files and updates to example configurations, including increasing the prefix_length for private service access and fixes for NVIDIA driver installation. A security concern was identified in the import_pytorch_container.sh scripts, where insecure temporary directory creation in /tmp could lead to potential interference in multi-user environments. It is recommended to use mktemp -d for secure and unique directory creation, aligning with repository guidelines for secure shell scripting practices.
| if [ ! -d "$XDG_RUNTIME_DIR" ]; then | ||
| # Fallback to a guaranteed writable location in /tmp | ||
| XDG_RUNTIME_DIR="/tmp/enroot-runtime-$(id -u)" | ||
| export XDG_RUNTIME_DIR | ||
| mkdir -p "$XDG_RUNTIME_DIR" | ||
| chmod 700 "$XDG_RUNTIME_DIR" | ||
| fi |
There was a problem hiding this comment.
The script creates a temporary directory in /tmp using a predictable name based on the user's UID, which is insecure and vulnerable to symlink attacks in a multi-user environment. An attacker could pre-create this directory with insecure permissions to intercept or interfere with the enroot import process. Additionally, the script lacks set -e, meaning it will continue to execute even if chmod 700 fails. It is recommended to use mktemp -d for secure and unique directory creation.
| if [ ! -d "$XDG_RUNTIME_DIR" ]; then | |
| # Fallback to a guaranteed writable location in /tmp | |
| XDG_RUNTIME_DIR="/tmp/enroot-runtime-$(id -u)" | |
| export XDG_RUNTIME_DIR | |
| mkdir -p "$XDG_RUNTIME_DIR" | |
| chmod 700 "$XDG_RUNTIME_DIR" | |
| fi | |
| if [ ! -d "$XDG_RUNTIME_DIR" ]; then | |
| # Fallback to a guaranteed writable location in /tmp | |
| XDG_RUNTIME_DIR=$(mktemp -d -t enroot-runtime-$(id -u).XXXXXX) | |
| export XDG_RUNTIME_DIR | |
| fi |
References
- In shell scripts, avoid creating temporary directories in
/tmpwith predictable names and world-writable permissions, especially withsudo, as this is vulnerable to symlink attacks. Prefer usingmktemp -dfor secure temporary directory creation, or usemkdirwith restrictive permissions (e.g.,700) and withoutsudo.
| if [ ! -d "$XDG_RUNTIME_DIR" ]; then | ||
| # Fallback to a guaranteed writable location in /tmp | ||
| XDG_RUNTIME_DIR="/tmp/enroot-runtime-$(id -u)" | ||
| export XDG_RUNTIME_DIR | ||
| mkdir -p "$XDG_RUNTIME_DIR" | ||
| chmod 700 "$XDG_RUNTIME_DIR" | ||
| fi |
There was a problem hiding this comment.
The script creates a temporary directory in /tmp using a predictable name based on the user's UID, which is insecure and vulnerable to symlink attacks in a multi-user environment. An attacker could pre-create this directory with insecure permissions to intercept or interfere with the enroot import process. Additionally, the script lacks set -e, meaning it will continue to execute even if chmod 700 fails. It is recommended to use mktemp -d for secure and unique directory creation.
| if [ ! -d "$XDG_RUNTIME_DIR" ]; then | |
| # Fallback to a guaranteed writable location in /tmp | |
| XDG_RUNTIME_DIR="/tmp/enroot-runtime-$(id -u)" | |
| export XDG_RUNTIME_DIR | |
| mkdir -p "$XDG_RUNTIME_DIR" | |
| chmod 700 "$XDG_RUNTIME_DIR" | |
| fi | |
| if [ ! -d "$XDG_RUNTIME_DIR" ]; then | |
| # Fallback to a guaranteed writable location in /tmp | |
| XDG_RUNTIME_DIR=$(mktemp -d -t enroot-runtime-$(id -u).XXXXXX) | |
| export XDG_RUNTIME_DIR | |
| fi |
References
- In shell scripts, avoid creating temporary directories in
/tmpwith predictable names and world-writable permissions, especially withsudo, as this is vulnerable to symlink attacks. Prefer usingmktemp -dfor secure temporary directory creation, or usemkdirwith restrictive permissions (e.g.,700) and withoutsudo.
Merge V1.83.0 into Develop
Submission Checklist
NOTE: Community submissions can take up to 2 weeks to be reviewed.
Please take the following actions before submitting this pull request.