fix: address critical and high CVE findings - #1103
Open
mckornfield wants to merge 3 commits into
Open
Conversation
Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>
Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>
Contributor
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe changes raise dependency minimums and update Docker image remediation. Runtime builds now install the CVE cleanup script, pin Pillow, and remove stale Pillow files. ChangesDependency and image remediation
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
CVE_SUMMARY.md (1)
1-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the required documentation structure.
This page records facts and verification evidence, so classify it as REFERENCE. Put a short status summary and a
## Prerequisitessection before## Artifacts Reviewed. Add a## Next Stepssection after## Verificationfor the required image rescan and Studio tests.As per coding guidelines, Markdown pages must fit one Diataxis quadrant, list prerequisites at the top, and include a
Next Stepssection at the end.Also applies to: 96-97
🤖 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 `@CVE_SUMMARY.md` around lines 1 - 5, Add the required REFERENCE-page structure in CVE_SUMMARY.md: place a concise status summary and a ## Prerequisites section before ## Artifacts Reviewed, and add ## Next Steps after ## Verification describing the required image rescan and Studio tests. Keep the page focused on factual evidence and verification.Source: Coding guidelines
🤖 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 `@CVE_SUMMARY.md`:
- Around line 77-78: Update the verification command in CVE_SUMMARY.md to invoke
summarize_findings.py through a repository-relative path or another portable
tool location, removing the host-specific /home/mkornfield/home/skills prefix.
Verify the documented command works from a fresh checkout before retaining it.
- Around line 49-51: Update the `nmp-cpu-tasks`, `nmp-automodel-training`, and
`nmp-unsloth-training` entries in `CVE_SUMMARY.md` to describe their
remediations as expected or pending rather than confirmed. Reserve definitive
“addressed” wording until rebuilt images receive a fresh Pulse scan confirming
the package fixes and stale-file removal.
In `@docker/scripts/cve-cleanup.sh`:
- Around line 90-92: Update the Pillow cleanup command in the CVE cleanup script
to remove the packages from the active /app/.venv Python 3.13 environment
instead of the Python 3.12 system paths, covering the Pillow package, native
libraries, and dist-info metadata; then verify the rebuilt CPU image no longer
contains the vulnerable installation.
In `@pyproject.toml`:
- Line 269: Update the Automodel image dependency constraints in its Dockerfile
to require GitPython>=3.1.57,<4 and pyasn1>=0.6.4,<1, matching the project
dependency floors. Rebuild the image and run the image security scan to verify
the updated constraints.
---
Nitpick comments:
In `@CVE_SUMMARY.md`:
- Around line 1-5: Add the required REFERENCE-page structure in CVE_SUMMARY.md:
place a concise status summary and a ## Prerequisites section before ##
Artifacts Reviewed, and add ## Next Steps after ## Verification describing the
required image rescan and Studio tests. Keep the page focused on factual
evidence and verification.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ed093274-8705-4a3c-b777-6cc53d020f37
⛔ Files ignored due to path filters (3)
services/guardrails/callouts/go.sumis excluded by!**/*.sumuv.lockis excluded by!**/*.lockweb/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
CVE_SUMMARY.mddocker/Dockerfile.auditor-tasksdocker/Dockerfile.nmp-cpu-tasksdocker/Dockerfile.nmp-unsloth-trainingdocker/automodel/Dockerfile.nmp-automodel-basedocker/scripts/cve-cleanup.shpyproject.tomlservices/core/models/pyproject.tomlservices/guardrails/callouts/go.modweb/pnpm-workspace.yaml
Comment on lines
+49
to
+51
| - `nmp-cpu-tasks` Perl package rows: 12 Critical and 16 High rows across `perl-base`, `perl`, `perl-modules-5.36`, and `libperl5.36` are addressed by the runtime cleanup step. | ||
| - `nmp-automodel-training` Pillow rows: 10 High rows for stale `/usr/local/lib/python3.12/dist-packages/pillow-12.2.0` copies are addressed by removing shadowed system-site Pillow. | ||
| - `nmp-unsloth-training` Pillow rows: 10 High rows for stale `/usr/local/lib/python3.12/dist-packages/pillow-12.2.0` copies are addressed by installing Pillow `>=12.3.0,<13` into `/opt/venv` and removing the shadowed system copy. |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/sh
set -eu
file=$(git ls-files | awk '$0 == "CVE_SUMMARY.md" {print; exit}')
test -n "$file"
printf '%s\n' '--- CVE_SUMMARY.md (relevant sections) ---'
sed -n '1,115p' "$file"
printf '%s\n' '--- references to rebuild, rescan, addressed, and Pulse ---'
rg -n -i 'rebuild|rescan|addressed|expected|Pulse|scan' "$file"Repository: NVIDIA-NeMo/nemo-platform
Length of output: 163
🏁 Script executed:
#!/bin/sh
set -u
printf '%s\n' '--- repository root ---'
pwd
printf '%s\n' '--- tracked matching files ---'
git ls-files | grep -E '(^|/)CVE_SUMMARY\.md$' || true
printf '%s\n' '--- candidate file locations ---'
find . -type f -name 'CVE_SUMMARY.md' -print
printf '%s\n' '--- candidate file content ---'
for file in $(find . -type f -name 'CVE_SUMMARY.md' -print); do
sed -n '1,115p' "$file"
printf '%s\n' '--- references ---'
grep -nEi 'rebuild|rescan|addressed|expected|Pulse|scan' "$file" || true
doneRepository: NVIDIA-NeMo/nemo-platform
Length of output: 301
Security Misconfiguration (CWE-693)
Reachability: Internal · Exploitability: Moderate
Separate intended remediation from scan-confirmed remediation.
Because Lines 49–51 state that the findings are addressed while Line 96 states that the images were not rebuilt or rescanned, change these entries to “expected to be addressed” or move them to a pending section. Close them only after a fresh Pulse scan confirms the fixed packages and removal of stale files.
🤖 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 `@CVE_SUMMARY.md` around lines 49 - 51, Update the `nmp-cpu-tasks`,
`nmp-automodel-training`, and `nmp-unsloth-training` entries in `CVE_SUMMARY.md`
to describe their remediations as expected or pending rather than confirmed.
Reserve definitive “addressed” wording until rebuilt images receive a fresh
Pulse scan confirming the package fixes and stale-file removal.
Comment on lines
+90
to
+92
| rm -rf /usr/local/lib/python3.12/dist-packages/PIL \ | ||
| /usr/local/lib/python3.12/dist-packages/pillow.libs \ | ||
| /usr/local/lib/python3.12/dist-packages/pillow-*.dist-info |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 2 'nmp-python-base|python:3\.[0-9]+' \
docker/Dockerfile.nmp-cpu-tasks docker CVE_SUMMARY.md
rg -n 'dist-packages|PIL|pillow' \
docker/scripts/cve-cleanup.sh docker --glob 'Dockerfile*'Repository: NVIDIA-NeMo/nemo-platform
Length of output: 3995
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- base image and shared script ---'
cat -n docker/base/Dockerfile.nmp-python-base | sed -n '1,90p'
cat -n docker/scripts/cve-cleanup.sh | sed -n '1,130p'
printf '%s\n' '--- cleanup callers and Pillow installation paths ---'
rg -n -C 3 'cve-cleanup\.sh|Pillow|pillow|dist-packages|site-packages|uv sync|pip install' dockerRepository: NVIDIA-NeMo/nemo-platform
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
base = Path("docker/base/Dockerfile.nmp-python-base").read_text()
cpu = Path("docker/Dockerfile.nmp-cpu-tasks").read_text()
cleanup = Path("docker/scripts/cve-cleanup.sh").read_text()
assert "python:3.13.14-slim-trixie" in base
assert "uv venv --seed --python=3.13 /app/.venv" in base
assert "uv sync --frozen --only-group cpu-tasks --no-editable" in cpu
assert "COPY docker/scripts/cve-cleanup.sh /bin/" in cpu
assert "RUN bash /bin/cve-cleanup.sh" in cpu
cleanup_paths = re.findall(r"/usr/local/lib/python[\d.]+/dist-packages/[^\s\\]+", cleanup)
print("base_python=3.13")
print("cpu_venv=/app/.venv")
print("expected_cpu_pillow_paths:")
for name in ("PIL", "pillow.libs", "pillow-*.dist-info"):
print(f" /app/.venv/lib/python3.13/site-packages/{name}")
print("cleanup_pillow_paths:")
for path in cleanup_paths:
if any(part in path.lower() for part in ("pil", "pillow")):
print(f" {path}")
print("cpu_pillow_paths_covered_by_cleanup=False")
PY
cat -n docker/Dockerfile.nmp-cpu-tasks | sed -n '1,30p'Repository: NVIDIA-NeMo/nemo-platform
Length of output: 1860
Security Misconfiguration (CWE-693)
Reachability: Internal · Exploitability: Theoretical
Use the active virtual environment for Pillow cleanup.
The CPU image installs Python 3.13 packages in /app/.venv, but the script deletes only Python 3.12 system packages. Remove Pillow from the active environment and verify the rebuilt image.
🤖 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 `@docker/scripts/cve-cleanup.sh` around lines 90 - 92, Update the Pillow
cleanup command in the CVE cleanup script to remove the packages from the active
/app/.venv Python 3.13 environment instead of the Python 3.12 system paths,
covering the Pillow package, native libraries, and dist-info metadata; then
verify the rebuilt CPU image no longer contains the vulnerable installation.
Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
security.ymlrun 31002650434CVE Notes
langchain-community<0.4is currently capped forragas 0.4.3compatibility, and the Pulse OSS report givesragasunknown upgrade guidance.Verification
uv lock --checkcd services/core/jobs/jobs-launcher && go test ./...cd services/guardrails/callouts && go test ./...cd web && pnpm install --lockfile-onlyrg -n "vitest@4\\.1\\.9|@vitest/(coverage-v8|ui)@4\\.1\\.9|vitest: \\^4\\.1\\.9|version: 4\\.1\\.9" web/pnpm-workspace.yaml web/pnpm-lock.yamlgit diff --check origin/main...HEADbash -n docker/scripts/cve-cleanup.shdocker buildx bake --print nmp-cpu-tasks-dockerCaveats
v22.18.0, below the repo requirement>=22.23.2 <23.web/node_moduleswas not cleaned or refreshed; only the lockfile was updated.Summary by CodeRabbit
Bug Fixes
Maintenance