fix: disable build isolation for rocm280 dependency install#934
Conversation
Flash-attn's build hooks import torch, which fails under pip build isolation during micropipenv installation in the py312-rocm64-torch280 image. Set PIP_NO_BUILD_ISOLATION=1 for the install step so torch is visible and the image build no longer fails at flash-attn resolution. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughIn Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Security note — CWE-829 (Inclusion of Functionality from Untrusted Control Sphere):
🚥 Pre-merge checks | ✅ 9 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (9 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@images/runtime/training/py312-rocm64-torch280/Dockerfile`:
- Line 59: Move the dependency installation in the Dockerfile away from root:
the `micropipenv install` step with `PIP_NO_BUILD_ISOLATION=1` is currently
executed as `USER 0`, so change the build flow to install packages under a
non-root user and keep any filesystem permission adjustment in a separate
privileged step. Use the existing Dockerfile build stages around the `RUN
PIP_NO_BUILD_ISOLATION=1 micropipenv install ...` instruction to locate and
split the install and permission-fix logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: d8c2eae1-f04d-469c-aabc-366fb7229812
📒 Files selected for processing (1)
images/runtime/training/py312-rocm64-torch280/Dockerfile
| COPY Pipfile.lock ./ | ||
|
|
||
| RUN micropipenv install -- --no-cache-dir && \ | ||
| RUN PIP_NO_BUILD_ISOLATION=1 micropipenv install -- --no-cache-dir && \ |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Major: Line 59 runs unisolated package build hooks as root (CWE-250).
PIP_NO_BUILD_ISOLATION=1 makes micropipenv install execute any sdist/PEP 517 build hook against the ambient site-packages, and this step still runs under USER 0. A compromised package in Pipfile.lock can therefore execute arbitrary code as root during the image build and mutate the system Python environment. Move the dependency install to a non-root user and keep the permission fix in a separate privileged step.
Remediation
-COPY Pipfile.lock ./
-
-RUN PIP_NO_BUILD_ISOLATION=1 micropipenv install -- --no-cache-dir && \
- rm -f ./Pipfile.lock && \
- # Fix permissions to support pip in OpenShift environments \
- chmod -R g+w /opt/app-root/lib/python3.12/site-packages && \
- fix-permissions /opt/app-root -P
+COPY --chown=1001:0 Pipfile.lock ./
+
+USER 1001
+RUN PIP_NO_BUILD_ISOLATION=1 micropipenv install -- --no-cache-dir && \
+ rm -f ./Pipfile.lock
+
+USER 0
+RUN chmod -R g+w /opt/app-root/lib/python3.12/site-packages && \
+ fix-permissions /opt/app-root -PAs per path instructions, **/Dockerfile*: Run as non-root user.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| RUN PIP_NO_BUILD_ISOLATION=1 micropipenv install -- --no-cache-dir && \ | |
| COPY --chown=1001:0 Pipfile.lock ./ | |
| USER 1001 | |
| RUN PIP_NO_BUILD_ISOLATION=1 micropipenv install -- --no-cache-dir && \ | |
| rm -f ./Pipfile.lock | |
| USER 0 | |
| RUN chmod -R g+w /opt/app-root/lib/python3.12/site-packages && \ | |
| fix-permissions /opt/app-root -P |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@images/runtime/training/py312-rocm64-torch280/Dockerfile` at line 59, Move
the dependency installation in the Dockerfile away from root: the `micropipenv
install` step with `PIP_NO_BUILD_ISOLATION=1` is currently executed as `USER 0`,
so change the build flow to install packages under a non-root user and keep any
filesystem permission adjustment in a separate privileged step. Use the existing
Dockerfile build stages around the `RUN PIP_NO_BUILD_ISOLATION=1 micropipenv
install ...` instruction to locate and split the install and permission-fix
logic.
Source: Path instructions
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sutaakar The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@kapil27: The following test has Failed: OCI Artifact Browser URLInspecting Test Artifacts ManuallyTo inspect your test artifacts manually, follow these steps:
mkdir -p oras-artifacts
cd oras-artifacts
oras pull quay.io/opendatahub/odh-ci-artifacts:odh-pr-test-distributed-workloads-t2kgs |
3 participants
Flash-attn's build hooks import torch, which fails under pip build isolation during micropipenv installation in the py312-rocm64-torch280 image. Set PIP_NO_BUILD_ISOLATION=1 for the install step so torch is visible and the image build no longer fails at flash-attn resolution.
Description
How Has This Been Tested?
Merge criteria:
Summary by CodeRabbit